feat(identity): ERC-8004 identity + reputation MCP tools

[arcayne]

Summary

• Adds 7 ERC-8004 identity MCP tools: agent_register, agent_update, agent_deregister, agent_status, agent_list, agent_reputation, agent_feedback
• Integrates agent card generation and IPFS (Pinata) upload; supports metadata fields (description, image, services, actions, supportedTrust, tags, version, license, sourceCode, documentation, x402, active)
• Converges onto @injective/agent-sdk for identity/reputation registry interaction; adds viem client factory, EIP-712 wallet-link helper, and error classes

Test plan

• pnpm typecheck clean
• pnpm build clean
• pnpm test — 326 passed / 6 skipped (integration suite runs via test:integration)
• Manual e2e smoke via scripts/create-x402-agent on testnet (reviewer)

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Added on-chain agent identity management: register new agents, update agent metadata, and deregister agents
  • Added agent identity queries: retrieve agent status, list agents with optional filtering, and check reputation scores
  • Added feedback capabilities: submit and revoke feedback on agents

• Documentation

  • Updated README with new Identity (ERC-8004) tools documentation

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a comprehensive ERC-8004 agent identity system featuring on-chain agent registration, status retrieval, listing, deregistration, and feedback management via a new identity module that wraps the @injective/agent-sdk, exposed through MCP tools and read-only handlers.

Changes

Cohort / File(s)	Summary
Configuration & Dependencies .gitignore, README.md, package.json	Updated build artifacts exclusion, added identity/ERC-8004 documentation to README with new MCP tools list, and added @injective/agent-sdk and viem dependencies.
Error Definitions src/errors/index.ts, src/errors/errors.test.ts	Added three new error classes (IdentityNotFound, IdentityTxFailed, DeregisterNotConfirmed) with tests validating error codes, names, and message formatting.
Identity Module src/identity/index.ts, src/identity/read.ts	Implemented identity write handler (register/update/deregister/feedback) and read handler (status/list/reputation/feedbackList) adapters over @injective/agent-sdk with wallet management, IPFS integration via Pinata, and error normalization.
Identity Tests src/identity/identity.test.ts, src/identity/read.test.ts, src/identity/demo.test.ts, src/identity/integration.test.ts	Added comprehensive unit and integration tests covering mocked SDK interactions, wallet linking logic, Pinata IPFS operations, and real testnet end-to-end scenarios with on-chain operations.
Integration Tests src/integration/identity.integration.test.ts	Added stateful end-to-end integration test exercising full agent lifecycle (register/status/update/list/deregister) with wallet import, address conversion, and transaction validation on testnet.
MCP Server Integration src/mcp/server.ts, src/mcp/server.test.ts	Extended MCP server with new "Identity Tools" group registering 9 tools (agent_register, agent_update, agent_deregister, agent_status, agent_list, agent_reputation, agent_feedback_list, agent_give_feedback, agent_revoke_feedback) and added validation schema tests for builder code, Ethereum addresses, and agent types.
Documentation src/identity/INTEGRATION.md	Added end-to-end integration test documentation describing test scope, environment setup, execution modes, troubleshooting, and prerequisites for testnet and Pinata IPFS operations.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A new identity system hops into place,
ERC-8004 agents join the blockchain race,
Registers, updates, and feedback so keen,
On-chain reputation keeping things clean!
With tests standing guard and MCP tools to guide,
The agent identity takes prideful stride. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title clearly and concisely summarizes the main change: adding ERC-8004 identity and reputation MCP tools, which is the primary focus across all modified files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/erc-8004-identity-tools

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	viem@​2.46.2 ⏵ 2.47.6	+1		+1

View full report

[coderabbitai]

Actionable comments posted: 8

♻️ Duplicate comments (1)

src/identity/index.ts (1)

271-291: ⚠️ Potential issue | 🟡 Minor

Root cause for the value integer issue flagged in server.ts.

BigInt(params.value) throws RangeError for any non-integer number. The schema fix in agent_give_feedback (z.number().int()) is the right place to enforce this; see the comment there. Noting here so it's clear why: this line is the source that makes the schema constraint necessary. Consider also defensively coercing with Math.trunc(params.value) if you want to tolerate rounding, but schema-level rejection is cleaner.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/index.ts` around lines 271 - 291, The BigInt(params.value) call
in giveFeedback is throwing RangeError for non-integer numbers; update the
giveFeedback implementation to either assume schema-level validation or
defensively coerce to an integer before BigInt: replace BigInt(params.value)
with BigInt(Math.trunc(params.value)) (or otherwise assert
Number.isInteger(params.value) and throw a clear error) in the giveFeedback
method that calls createClient so non-integer inputs no longer cause RangeError
at runtime.

🧹 Nitpick comments (5)

src/identity/integration.test.ts (1)

38-38: Nit: confirm describe.skipIf semantics with a string.

SKIP_REASON is string | null. describe.skipIf skips on truthy values, so this works in practice — but passing the reason string as the condition obscures intent. Consider passing a boolean and logging the reason separately, e.g.:

const shouldSkip = !TEST_ADDRESS || !TEST_PASSWORD || !PINATA_JWT
if (shouldSkip) console.log(`Skipping identity integration tests: ${SKIP_REASON}`)
describe.skipIf(shouldSkip)('Identity Integration Tests (Testnet)', ...)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/integration.test.ts` at line 38, The test currently passes
SKIP_REASON (a string|null) directly to describe.skipIf which relies on
truthiness and obscures intent; compute an explicit boolean (e.g., shouldSkip)
from the required checks (TEST_ADDRESS, TEST_PASSWORD, PINATA_JWT) and pass that
to describe.skipIf, and separately log SKIP_REASON when shouldSkip is true to
preserve the human-readable reason; update the test declaration using shouldSkip
instead of SKIP_REASON and add a console.log or test logger for SKIP_REASON when
skipping.

src/identity/demo.test.ts (1)

74-145: Heavy console.log output in a standard unit test will clutter every npm test run.

This file is picked up by the default vitest run (not gated behind an env var the way integration.test.ts is), so the emoji-rich log lines will print on every developer/CI run. Consider either:

• Gating the logs behind if (process.env.DEBUG) / process.env.VITEST_VERBOSE, or
• Moving this into the integration suite if it's intended as a walkthrough rather than a regression test, or
• Dropping the logs entirely and relying on the assertions (they fully cover the flow).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/demo.test.ts` around lines 74 - 145, The test emits many
emoji-rich console.log calls (e.g., lines around identity.register and
identityRead.status printing registerResult and statusResult) which will clutter
every vitest run; either remove those console.log statements, or wrap them with
a runtime guard such as if (process.env.VITEST_VERBOSE || process.env.DEBUG)
before the blocks that log the registration and status outputs, or move this
entire test into the integration suite; update references to the mocked call
assertions (mockRegister) to remain unchanged and ensure the test still asserts
registerResult and statusResult values after removing/gating logs.

README.md (1)

156-166: Optional: architecture tree omits the new identity/ module.

Consider adding identity/ ERC-8004 agent identity + reputation (wraps @injective/agent-sdk) to the tree so the diagram stays in sync with the added surface.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 156 - 166, Update the architecture tree in the README
to include the new identity module: add a line like "├── identity/    ERC-8004
agent identity + reputation (wraps `@injective/agent-sdk`)" in the same list where
other modules (config, keystore, wallets, markets, accounts, trading,
evm/eip712, orders, transfers, bridges, evm) are declared so the diagram and
module surface reflect the added identity/ module.

src/identity/read.ts (1)

92-113: Optional: 3× over-fetch for type filtering is fragile when the requested type is rare.

If only ~5% of agents on the registry match type='analytics' and the caller asks for limit: 20, fetchLimit = 60 will likely yield fewer than 20 matches and the caller has no way to know whether they got a truncated page. total here is post-filter count from the over-fetched slice, not the true total matching the filter on-chain.

Given the SDK doesn't index by type, this is a pragmatic approach, but consider either:

• Documenting the caveat in the tool description (results are best-effort when type is set), or
• Paginating until limit is reached or the SDK returns fewer than fetchLimit.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/read.ts` around lines 92 - 113, The current 3x over-fetch using
fetchLimit when params.type is set can underdeliver when the type is rare;
change the logic in the read handler that calls sdk.getAgentsByOwner /
sdk.listAgents to page and accumulate results until either you have at least
limit matching agents (filtering by params.type) or the SDK returns fewer items
than the page size (no more data), then slice to limit and set filteredTotal to
the total matching count you accumulated (not just the sliced length); ensure
you preserve the existing agents mapping (agentId, name, agentType, owner) and
stop paginating when no more pages are returned to avoid infinite loops.

src/mcp/server.ts (1)

985-985: Minor: owner schema accepts arbitrary strings.

identityRead.list only handles two shapes: a string starting with inj (converted via evm.injAddressToEth) or anything else passed through as a 0x address. Any other input (e.g., a typo, an ENS name, empty string) is forwarded to sdk.getAgentsByOwner as a malformed hex address and surfaces as a cryptic SDK error.

Recommend validating at the schema layer to match the tool description (inj1... or 0x...):

🛡️ Proposed fix

-    owner: z.string().optional().describe('Filter by owner — accepts inj1... or 0x... address.'),
+    owner: z.string()
+      .regex(/^(inj1[a-z0-9]{38}|0x[a-fA-F0-9]{40})$/, 'Must be inj1... or 0x... address')
+      .optional()
+      .describe('Filter by owner — accepts inj1... or 0x... address.'),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/mcp/server.ts` at line 985, The owner schema currently allows arbitrary
strings; restrict it to only the two accepted shapes so identityRead.list won't
forward invalid values to sdk.getAgentsByOwner. Replace z.string().optional()
for the owner field with a union/refinement that accepts either an inj-prefixed
address (string starting with "inj") or a 0x hex Ethereum address (e.g., regex
/^0x[0-9a-fA-F]{40}$/), and keep it optional; mention the same symbols in the
code (the owner schema field and the identityRead.list flow) so callers are
validated at the schema layer before evm.injAddressToEth or sdk.getAgentsByOwner
are invoked.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Line 22: The package.json currently uses an unresolved local file dependency
"@injective/agent-sdk": "file:../injective-agent-cli/packages/sdk" which breaks
CI and npm installs; replace that entry with a machine-independent reference —
e.g. if you move both repos into a monorepo, change to "workspace:*" and add a
pnpm-workspace.yaml; or publish `@injective/agent-sdk` and pin a published version
(e.g. "0.x.y"); or point to the git URL with path
(git+https://github.com/<org>/injective-agent-cli.git#<tag>&path=/packages/sdk)
— update the "@injective/agent-sdk" value accordingly and ensure CI workflows
reflect the chosen approach.

In `@README.md`:
- Around line 59-67: Add the three missing identity tools to the Identity
(ERC-8004) table and add a blank line after the heading to satisfy MD058: insert
rows for `agent_reputation` (read-only: returns aggregated reputation — score,
count, clients), `agent_feedback_list` (read-only: lists individual feedback
entries with value, tags, revocation status; no gas), and `agent_give_feedback`
(on-chain feedback submission; costs gas), and ensure there is an empty line
between the "### Identity (ERC-8004)" heading and the table header so
markdownlint MD058 is resolved.

In `@src/identity/demo.test.ts`:
- Around line 1-8: Update the top-of-file doc comment to reference the correct
test filename: change the run command "npm test -- src/identity/demo.ts" to "npm
test -- src/identity/demo.test.ts" (edit the comment block at the top of
demo.test.ts). Ensure the example command matches the actual test filename so
the run pattern works as expected.

In `@src/identity/INTEGRATION.md`:
- Around line 91-97: Update the "Increase Timeout" section so the shown npm test
command actually increases Vitest's per-test timeout: replace or augment the
suggested flags with Vitest's timeout flag (for example use
--testTimeout=300000) or explicitly instruct readers to edit the timeout in the
test file src/identity/integration.test.ts; mention the exact test file name and
the --testTimeout=300000 flag so consumers know where to change it or how to run
tests with a longer timeout.

In `@src/identity/integration.test.ts`:
- Around line 38-89: The tests leak state and rely on cross-test shared variable
agentId set inside the "it('registers agent on testnet with full metadata')"
test; move registration into a deterministic setup and add cleanup: register the
agent in beforeAll (call identity.register with TEST_ADDRESS/TEST_PASSWORD to
produce agentId and keep the existing assertions as separate it()s if you want
reporting) or collapse into a single e2e lifecycle it(); then add an afterAll
that calls identity.deregister(agentId, { confirm: true }) (or the equivalent
deregister function) as a best-effort cleanup to remove the on-chain agent;
update imports to include afterAll from vitest if needed and reference the
agentId variable produced in setup for all subsequent tests.

In `@src/identity/read.ts`:
- Around line 115-159: Both reputation and feedbackList currently swallow all
errors; update the catch blocks in the reputation and feedbackList methods to
log the error (include agentId and error.message) and only return the zero/empty
result for the SDK's "no entries"/"not found" condition; for all other
exceptions rethrow. Locate the reputation method (calls getReputation) and
feedbackList method (calls getFeedbackEntries) and replace the broad catch(_err)
with logic that (1) captures the thrown error, (2) checks the
SDK/registry-specific "not found" or "no entries" error sentinel/class/string
and if matched returns the current zeroed result, otherwise call
processLogger.error or similar with a structured message including agentId and
the error, then rethrow the error.

In `@src/integration/identity.integration.test.ts`:
- Around line 95-103: The test is flaky because identityRead.list(config, {
limit: 50 }) can miss the newly created agent; update the call to scope results
to the test wallet by passing the owner filter (e.g. identityRead.list(config, {
limit: 50, owner: <wallet address> })), using the existing config wallet/address
field so the search reliably returns agents for this wallet; then assert found
by agentId as before (references: identityRead.list, agentId, config).

In `@src/mcp/server.ts`:
- Around line 1038-1039: The schema currently allows fractional numbers for the
rating `value` which then causes `identity.giveFeedback` to throw when it calls
BigInt(params.value); change the schema entry named `value` in the relevant Zod
object (the lines defining `value: z.number()...`) to enforce integers (e.g.,
use `.int()` like `valueDecimals` does) so non-integer inputs are rejected early
and produce a validation error before `giveFeedback` runs.

---

Duplicate comments:
In `@src/identity/index.ts`:
- Around line 271-291: The BigInt(params.value) call in giveFeedback is throwing
RangeError for non-integer numbers; update the giveFeedback implementation to
either assume schema-level validation or defensively coerce to an integer before
BigInt: replace BigInt(params.value) with BigInt(Math.trunc(params.value)) (or
otherwise assert Number.isInteger(params.value) and throw a clear error) in the
giveFeedback method that calls createClient so non-integer inputs no longer
cause RangeError at runtime.

---

Nitpick comments:
In `@README.md`:
- Around line 156-166: Update the architecture tree in the README to include the
new identity module: add a line like "├── identity/    ERC-8004 agent identity +
reputation (wraps `@injective/agent-sdk`)" in the same list where other modules
(config, keystore, wallets, markets, accounts, trading, evm/eip712, orders,
transfers, bridges, evm) are declared so the diagram and module surface reflect
the added identity/ module.

In `@src/identity/demo.test.ts`:
- Around line 74-145: The test emits many emoji-rich console.log calls (e.g.,
lines around identity.register and identityRead.status printing registerResult
and statusResult) which will clutter every vitest run; either remove those
console.log statements, or wrap them with a runtime guard such as if
(process.env.VITEST_VERBOSE || process.env.DEBUG) before the blocks that log the
registration and status outputs, or move this entire test into the integration
suite; update references to the mocked call assertions (mockRegister) to remain
unchanged and ensure the test still asserts registerResult and statusResult
values after removing/gating logs.

In `@src/identity/integration.test.ts`:
- Line 38: The test currently passes SKIP_REASON (a string|null) directly to
describe.skipIf which relies on truthiness and obscures intent; compute an
explicit boolean (e.g., shouldSkip) from the required checks (TEST_ADDRESS,
TEST_PASSWORD, PINATA_JWT) and pass that to describe.skipIf, and separately log
SKIP_REASON when shouldSkip is true to preserve the human-readable reason;
update the test declaration using shouldSkip instead of SKIP_REASON and add a
console.log or test logger for SKIP_REASON when skipping.

In `@src/identity/read.ts`:
- Around line 92-113: The current 3x over-fetch using fetchLimit when
params.type is set can underdeliver when the type is rare; change the logic in
the read handler that calls sdk.getAgentsByOwner / sdk.listAgents to page and
accumulate results until either you have at least limit matching agents
(filtering by params.type) or the SDK returns fewer items than the page size (no
more data), then slice to limit and set filteredTotal to the total matching
count you accumulated (not just the sliced length); ensure you preserve the
existing agents mapping (agentId, name, agentType, owner) and stop paginating
when no more pages are returned to avoid infinite loops.

In `@src/mcp/server.ts`:
- Line 985: The owner schema currently allows arbitrary strings; restrict it to
only the two accepted shapes so identityRead.list won't forward invalid values
to sdk.getAgentsByOwner. Replace z.string().optional() for the owner field with
a union/refinement that accepts either an inj-prefixed address (string starting
with "inj") or a 0x hex Ethereum address (e.g., regex /^0x[0-9a-fA-F]{40}$/),
and keep it optional; mention the same symbols in the code (the owner schema
field and the identityRead.list flow) so callers are validated at the schema
layer before evm.injAddressToEth or sdk.getAgentsByOwner are invoked.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 37fc7396-fe94-46bb-8aa5-9cbaa09b468a

📥 Commits

Reviewing files that changed from the base of the PR and between 747441e and 032030a.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (15)

• .gitignore
• README.md
• package.json
• src/errors/errors.test.ts
• src/errors/index.ts
• src/identity/INTEGRATION.md
• src/identity/demo.test.ts
• src/identity/identity.test.ts
• src/identity/index.ts
• src/identity/integration.test.ts
• src/identity/read.test.ts
• src/identity/read.ts
• src/integration/identity.integration.test.ts
• src/mcp/server.test.ts
• src/mcp/server.ts

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

pnpm file: protocol dependency pointing to a sibling directory outside the repo — behavior on npm publish and in CI

💡 Result:

Using a pnpm file: protocol dependency pointing to a sibling directory outside the repository (e.g., "file:../sibling-pkg") works locally for development if the sibling directory exists relative to the project root, as pnpm resolves relative file: paths from the project root/workspace root (with fixes in later versions for overrides). Changes in the sibling propagate via hard links. On npm publish: The file: specifier remains unchanged in the published package.json. Since pnpm publish packs only files specified in the "files" field or package dir (excluding node_modules), the sibling directory is not included in the tarball. The published package has an unresolvable "file:../sibling-pkg" dependency, causing install failures for consumers (ERR_PNPM_LINKED_PKG_DIR_NOT_FOUND or similar). In CI: Fails with ERR_PNPM_LINKED_PKG_DIR_NOT_FOUND because the sibling directory outside the repo is not checked out/cloned in CI. The relative path resolves to a non-existent location. Recommendations: Use for local dev only. For publishable packages, use version ranges or workspace: (if in monorepo). In CI, use overrides with vendored tarballs (pnpm pack the sibling and reference "file:./vendor/sibling.tgz" with vendor dir in repo). Ensure lockfile compatibility across OS (resolved in recent pnpm versions).

Citations:

• 1: https://pnpm.io/cli/link
• 2: https://pnpm.io/package-sources
• 3: https://stackoverflow.com/questions/72032028/can-pnpm-replace-npm-link-yarn-link
• 4: readPackage / pnpm.overrides and relative paths in file: protocol dependencies pnpm/pnpm#5493
• 5: Fix (or document changes in) "file:" protocol pnpm/pnpm#8053
• 6: ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY with relative local dependency (difference between windows and linux CI?) pnpm/pnpm#7955
• 7: https://pnpm.io/cli/publish

---

Local file: dependency outside the repo breaks installs in CI and on npm publish.

"@injective/agent-sdk": "file:../injective-agent-cli/packages/sdk" points to a sibling directory that exists only on your machine. When published to npm or installed in CI, the sibling directory doesn't exist at that path, causing ERR_PNPM_LINKED_PKG_DIR_NOT_FOUND failures. Relative file: paths are resolved from the project root; outside-repo paths become unresolvable across machines.

Choose one:

• pnpm workspace (preferred if repos are developed together): Merge into a monorepo with pnpm-workspace.yaml and reference as "workspace:*".
• Published package: Publish @injective/agent-sdk to npm or a private registry and pin a version.
• Git dependency: Use "git+https://github.com/<org>/injective-agent-cli.git#<tag>&path=/packages/sdk" (pnpm supports git with path) or a committed tarball for reproducibility.

Until resolved, pnpm install on a fresh clone and CI workflows will fail.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 22, The package.json currently uses an unresolved local
file dependency "@injective/agent-sdk":
"file:../injective-agent-cli/packages/sdk" which breaks CI and npm installs;
replace that entry with a machine-independent reference — e.g. if you move both
repos into a monorepo, change to "workspace:*" and add a pnpm-workspace.yaml; or
publish `@injective/agent-sdk` and pin a published version (e.g. "0.x.y"); or
point to the git URL with path
(git+https://github.com/<org>/injective-agent-cli.git#<tag>&path=/packages/sdk)
— update the "@injective/agent-sdk" value accordingly and ensure CI workflows
reflect the chosen approach.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Extract the exact identity tool names registered in server.ts
rg -nP '\bserver\.tool\s*\(\s*["'\'']agent_[a-z_]+' --type=ts -g 'src/mcp/server.ts'

Repository: InjectiveLabs/mcp-server

Length of output: 50

---

🏁 Script executed:

#!/bin/bash
# Find server.ts file
fd -t f -name 'server.ts' | head -20

Repository: InjectiveLabs/mcp-server

Length of output: 298

---

🏁 Script executed:

#!/bin/bash
# Also check the repository structure
git ls-files | grep -E '(server|mcp)' | head -20

Repository: InjectiveLabs/mcp-server

Length of output: 117

---

🏁 Script executed:

#!/bin/bash
# Check README content at the specified lines
sed -n '59,67p' README.md

Repository: InjectiveLabs/mcp-server

Length of output: 575

---

🏁 Script executed:

#!/bin/bash
# Read the server.ts file to see tool registrations
cat -n src/mcp/server.ts | head -100

Repository: InjectiveLabs/mcp-server

Length of output: 5378

---

🏁 Script executed:

#!/bin/bash
# Search for all agent tool registrations in server.ts
rg 'server\.tool|agent_' src/mcp/server.ts -A 3 | head -80

Repository: InjectiveLabs/mcp-server

Length of output: 2615

---

🏁 Script executed:

#!/bin/bash
# Specifically search for agent_feedback and agent_reputation
rg 'agent_feedback|agent_reputation' src/mcp/server.ts

Repository: InjectiveLabs/mcp-server

Length of output: 257

---

🏁 Script executed:

#!/bin/bash
# Get all agent_* tools in server.ts with full context
rg "server\.tool\(\s*['\"]agent_" src/mcp/server.ts -A 2 | head -120

Repository: InjectiveLabs/mcp-server

Length of output: 50

---

🏁 Script executed:

#!/bin/bash
# Also search for agent_give_feedback since it's mentioned in the feedback_list context
rg 'agent_give|agent_feedback|agent_reputation|agent_register|agent_update|agent_deregister|agent_status|agent_list' src/mcp/server.ts -B 2 | head -150

Repository: InjectiveLabs/mcp-server

Length of output: 889

---

🏁 Script executed:

#!/bin/bash
# Get full descriptions of the missing tools for accurate rewrite
rg "server\.tool\(\s*['\"]agent_reputation|agent_feedback_list|agent_give_feedback" src/mcp/server.ts -A 10 | head -80

Repository: InjectiveLabs/mcp-server

Length of output: 2111

---

Identity tool table is missing three tools (agent_reputation, agent_feedback_list, agent_give_feedback), and MD058 requires a blank line before the table.

The README documents only five identity tools, but src/mcp/server.ts registers eight. The missing tools are:

• agent_reputation — Get aggregated reputation (score, count, clients) for an agent. Read-only.
• agent_feedback_list — List individual feedback entries for an agent with value, tags, and revocation status. Read-only, no gas cost.
• agent_give_feedback — Submit on-chain feedback for an agent. Costs gas.

Also, markdownlint MD058 at line 60 — add a blank line after the heading and before the table column definitions.

📝 Proposed fix

 ### Identity (ERC-8004)
+
 | Tool | Description |
 |---|---|
 | `agent_register` | Register a new AI agent identity on the ERC-8004 registry. Costs gas. |
 | `agent_update` | Update agent metadata, token URI, or linked wallet. Costs gas. |
 | `agent_deregister` | Permanently burn an agent's identity NFT (irreversible). Costs gas. |
 | `agent_status` | Get full agent details: metadata, wallet, owner, reputation. Read-only. |
 | `agent_list` | Find registered agents with optional owner/type filters. Read-only. |
+| `agent_reputation` | Get aggregated reputation (score, count, clients) for an agent. Read-only. |
+| `agent_feedback_list` | List individual feedback entries for an agent with value, tags, and revocation status. Read-only. |
+| `agent_give_feedback` | Submit on-chain feedback for an agent. Costs gas. |

🧰 Tools

🪛 markdownlint-cli2 (0.22.0)

[warning] 60-60: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 59 - 67, Add the three missing identity tools to the
Identity (ERC-8004) table and add a blank line after the heading to satisfy
MD058: insert rows for `agent_reputation` (read-only: returns aggregated
reputation — score, count, clients), `agent_feedback_list` (read-only: lists
individual feedback entries with value, tags, revocation status; no gas), and
`agent_give_feedback` (on-chain feedback submission; costs gas), and ensure
there is an empty line between the "### Identity (ERC-8004)" heading and the
table header so markdownlint MD058 is resolved.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Doc comment has a stale filename.

Line 7 says npm test -- src/identity/demo.ts but the file is demo.test.ts. The pattern won't match.

✏️ Fix

- *   npm test -- src/identity/demo.ts
+ *   npm test -- src/identity/demo.test.ts

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	/**
	* End-to-end demo: register an agent with full metadata, then fetch its status with reputation.
	*
	* This is a Vitest test file demonstrating the full identity workflow with mocked SDK.
	*
	* Run with:
	* npm test -- src/identity/demo.ts
	*/
	/**
	* End-to-end demo: register an agent with full metadata, then fetch its status with reputation.
	*
	* This is a Vitest test file demonstrating the full identity workflow with mocked SDK.
	*
	* Run with:
	* npm test -- src/identity/demo.test.ts
	*/

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/demo.test.ts` around lines 1 - 8, Update the top-of-file doc
comment to reference the correct test filename: change the run command "npm test
-- src/identity/demo.ts" to "npm test -- src/identity/demo.test.ts" (edit the
comment block at the top of demo.test.ts). Ensure the example command matches
the actual test filename so the run pattern works as expected.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Minor: "Increase Timeout" section shows a command that doesn't increase timeout.

--bail 1 --reporter=verbose controls failure-bail and reporter verbosity — it does not change the per-test timeout. Consider replacing with the Vitest timeout flag (e.g., --testTimeout=300000) or clarify that the timeout must be edited in the test file itself.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/INTEGRATION.md` around lines 91 - 97, Update the "Increase
Timeout" section so the shown npm test command actually increases Vitest's
per-test timeout: replace or augment the suggested flags with Vitest's timeout
flag (for example use --testTimeout=300000) or explicitly instruct readers to
edit the timeout in the test file src/identity/integration.test.ts; mention the
exact test file name and the --testTimeout=300000 flag so consumers know where
to change it or how to run tests with a longer timeout.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Integration suite leaks testnet state and is fragile to test filtering / reordering.

Two related concerns that will bite you after a few CI runs:

1. No cleanup. Every successful run registers a fresh agent on testnet and never deregisters it. Gas is wasted and the registry accumulates garbage indefinitely. The PR description says deregister is covered with confirm: true, but there’s no corresponding it(...) here. Add a final step (either an afterAll best-effort cleanup, or an it('deregisters the agent', ...) that asserts the tx and closes the lifecycle).

2. Cross-test shared state. agentId is assigned inside the first it and read by every subsequent it. If a user runs a single test via -t 'fetches registered agent', or the register test is skipped/fails, the downstream tests will silently use undefined as agentId, producing misleading failures (e.g. "newAgent is undefined" rather than "register failed"). Prefer either:

   • Doing the one-time setup in beforeAll (register there, keep the assertions as separate its for reporting), or
   • Consolidating the full lifecycle into a single it('e2e lifecycle', ...) so all steps share a deterministic scope.

🧹 Sketch: cleanup via afterAll

-  beforeAll(async () => {
+  beforeAll(async () => {
     console.log(`\n🔐 Verifying wallet at ${TEST_ADDRESS}...`)
     wallets.unlock(TEST_ADDRESS!, TEST_PASSWORD!)
     evmAddress = evm.injAddressToEth(TEST_ADDRESS!)
     console.log('✅ Wallet unlocked successfully\n')
   })
+
+  afterAll(async () => {
+    if (!agentId) return
+    try {
+      await identity.deregister(config, {
+        address: TEST_ADDRESS!,
+        password: TEST_PASSWORD!,
+        agentId,
+        confirm: true,
+      })
+    } catch (err) {
+      console.warn(`⚠️  Failed to deregister test agent ${agentId}:`, err)
+    }
+  })

(Remember to add afterAll to the vitest import.)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/integration.test.ts` around lines 38 - 89, The tests leak state
and rely on cross-test shared variable agentId set inside the "it('registers
agent on testnet with full metadata')" test; move registration into a
deterministic setup and add cleanup: register the agent in beforeAll (call
identity.register with TEST_ADDRESS/TEST_PASSWORD to produce agentId and keep
the existing assertions as separate it()s if you want reporting) or collapse
into a single e2e lifecycle it(); then add an afterAll that calls
identity.deregister(agentId, { confirm: true }) (or the equivalent deregister
function) as a best-effort cleanup to remove the on-chain agent; update imports
to include afterAll from vitest if needed and reference the agentId variable
produced in setup for all subsequent tests.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Minor: silent catch in reputation and feedbackList hides operational failures.

Both handlers swallow every error and return zeroed/empty results. That conflates three distinct states into one:

1. Agent genuinely has no feedback yet (expected happy path).
2. RPC endpoint is down or rate-limited.
3. Contract address misconfigured / not deployed for this network.

Callers and operators can't distinguish these. At minimum, log the error (structured, with agentId and error message) so broken-registry incidents aren't invisible. Better: narrow the swallow to the SDK's "no entries"/"not found" error class and rethrow the rest.

♻️ Suggested shape

-    } catch (_err) {
-      // Reputation is best-effort — return zeros if the agent has no feedback or registry errors
-      return { agentId: params.agentId, score: 0, count: 0, clients: [] }
-    }
+    } catch (err) {
+      const msg = err instanceof Error ? err.message : String(err)
+      // Only treat "not found" / "no entries" as the zero-feedback case
+      if (/nonexistent|not found|no entries|invalid token/i.test(msg)) {
+        return { agentId: params.agentId, score: 0, count: 0, clients: [] }
+      }
+      throw err
+    }

Same pattern applies to feedbackList at lines 155-158.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/identity/read.ts` around lines 115 - 159, Both reputation and
feedbackList currently swallow all errors; update the catch blocks in the
reputation and feedbackList methods to log the error (include agentId and
error.message) and only return the zero/empty result for the SDK's "no
entries"/"not found" condition; for all other exceptions rethrow. Locate the
reputation method (calls getReputation) and feedbackList method (calls
getFeedbackEntries) and replace the broad catch(_err) with logic that (1)
captures the thrown error, (2) checks the SDK/registry-specific "not found" or
"no entries" error sentinel/class/string and if matched returns the current
zeroed result, otherwise call processLogger.error or similar with a structured
message including agentId and the error, then rethrow the error.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm how sdk.listAgents orders results — if asc by agentId, the flakiness above is real.
fd -t f -e ts -e js 'listAgents' | head -n 20
rg -nP --type=ts -C4 '\blistAgents\s*\(' | head -n 200

Repository: InjectiveLabs/mcp-server

Length of output: 1205

---

🏁 Script executed:

# Read the integration test to see the exact context and full test
head -n 110 src/integration/identity.integration.test.ts | tail -n 25

Repository: InjectiveLabs/mcp-server

Length of output: 893

---

🏁 Script executed:

# Check the list() method signature in identity/read.ts
rg -nP --type=ts 'list\s*\(' src/identity/read.ts -A 10 | head -n 50

Repository: InjectiveLabs/mcp-server

Length of output: 506

---

🏁 Script executed:

# Look for type definitions or interface for list() method
rg -nP --type=ts 'interface.*List|type.*List' src/identity/read.ts -B 2 -A 5 | head -n 60

Repository: InjectiveLabs/mcp-server

Length of output: 1148

---

🏁 Script executed:

# Search for SDK agent ordering or sorting behavior
rg -nP --type=ts 'getAgentsByOwner|listAgents' src/ -A 5 | grep -iE 'sort|order|asc|desc' | head -n 20

Repository: InjectiveLabs/mcp-server

Length of output: 50

---

🏁 Script executed:

# Look for other integration tests that might use owner filtering
grep -rn 'identityRead.list' src/integration/ --include="*.ts"

Repository: InjectiveLabs/mcp-server

Length of output: 181

---

🏁 Script executed:

# Check the SDK client type for listAgents or getAgentsByOwner signatures
rg -nP 'interface.*AgentReadClient|type.*AgentReadClient' src/ -A 30 | head -n 80

Repository: InjectiveLabs/mcp-server

Length of output: 50

---

🏁 Script executed:

# Find SDK package dependency
rg -nP 'injective|agent' package.json

Repository: InjectiveLabs/mcp-server

Length of output: 394

---

🏁 Script executed:

# Find imports of AgentReadClient or SDK types
rg -nP "import.*from.*['\"]" src/identity/read.ts | head -n 10

Repository: InjectiveLabs/mcp-server

Length of output: 272

---

🏁 Script executed:

# Check if there's any SDK documentation or type hints in the repo
find . -name "*.d.ts" -o -name "*types*" | grep -i agent | head -n 10

Repository: InjectiveLabs/mcp-server

Length of output: 50

---

🏁 Script executed:

# Check test setup and see if there are any comments about testnet state or flakiness
head -n 50 src/integration/identity.integration.test.ts

Repository: InjectiveLabs/mcp-server

Length of output: 1878

---

🏁 Script executed:

# Search for retry mechanisms or flakiness handling in integration tests
rg -nP 'retry|flaky|timeout|maxRetries' src/integration/ --type=ts

Repository: InjectiveLabs/mcp-server

Length of output: 247

---

🏁 Script executed:

# Look at the beforeAll/beforeEach setup to understand test isolation
rg -nP 'beforeAll|beforeEach|describe.*identity' src/integration/identity.integration.test.ts -A 5 | head -n 40

Repository: InjectiveLabs/mcp-server

Length of output: 260

---

Minor: unfiltered listing may become flaky if testnet accumulates agents.

The test uses identityRead.list(config, { limit: 50 }) without filtering. If the SDK orders agents by ID ascending (or does not guarantee newest-first), the just-registered agent can fall outside the first 50 as testnet grows, causing expect(found).toBeDefined() to fail.

Scope the result to this wallet using the owner parameter, which is already supported:

🧪 Proposed fix

-    const result = await identityRead.list(config, { limit: 50 })
+    const result = await identityRead.list(config, { owner: testEvmAddress, limit: 50 })

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	it('lists agents and finds ours', async () => {
	const result = await identityRead.list(config, { limit: 50 })
	expect(result.agents.length).toBeGreaterThan(0)
	const found = result.agents.find(a => a.agentId === agentId)
	expect(found).toBeDefined()
	expect(found!.name).toBe('UpdatedTestBot')
	expect(found!.agentType).toBe('trading')
	}, 60_000)
	it('lists agents and finds ours', async () => {
	const result = await identityRead.list(config, { owner: testEvmAddress, limit: 50 })
	expect(result.agents.length).toBeGreaterThan(0)
	const found = result.agents.find(a => a.agentId === agentId)
	expect(found).toBeDefined()
	expect(found!.name).toBe('UpdatedTestBot')
	expect(found!.agentType).toBe('trading')
	}, 60_000)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/integration/identity.integration.test.ts` around lines 95 - 103, The test
is flaky because identityRead.list(config, { limit: 50 }) can miss the newly
created agent; update the call to scope results to the test wallet by passing
the owner filter (e.g. identityRead.list(config, { limit: 50, owner: <wallet
address> })), using the existing config wallet/address field so the search
reliably returns agents for this wallet; then assert found by agentId as before
(references: identityRead.list, agentId, config).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Minor: value should be .int() — BigInt conversion throws on fractional numbers.

identity.giveFeedback does BigInt(params.value) which throws RangeError: The number X cannot be converted to a BigInt because it is not an integer for any non-integer value. The description already states "integer", and neighboring valueDecimals uses .int(). Enforce it in the schema for a clean validation error:

🛡️ Proposed fix

-    value: z.number().describe('Rating value (integer). Meaning depends on your scale.'),
+    value: z.number().int().describe('Rating value (integer). Meaning depends on your scale.'),

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	value: z.number().describe('Rating value (integer). Meaning depends on your scale.'),
	valueDecimals: z.number().int().min(0).max(18).optional().describe('Decimal places for the value (default 0).'),
	value: z.number().int().describe('Rating value (integer). Meaning depends on your scale.'),
	valueDecimals: z.number().int().min(0).max(18).optional().describe('Decimal places for the value (default 0).'),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/mcp/server.ts` around lines 1038 - 1039, The schema currently allows
fractional numbers for the rating `value` which then causes
`identity.giveFeedback` to throw when it calls BigInt(params.value); change the
schema entry named `value` in the relevant Zod object (the lines defining
`value: z.number()...`) to enforce integers (e.g., use `.int()` like
`valueDecimals` does) so non-integer inputs are rejected early and produce a
validation error before `giveFeedback` runs.

[ckhbtc]

Review focused on the same funds-safety / irreversibility / LLM-trust-boundary lens as PR #10. Two blocking items, four tighten-before-merge items. No secret-leakage issues found; self-link wallet restriction and error-taxonomy additions look good.

Blocking

1. Optimistic tx-hash returns on every write path — no receipt confirmation.
2. confirm: boolean is LLM-supplied and not a real safety barrier for an irreversible NFT burn.

Should land in the same revision

3. agentId Zod schema accepts any non-empty string; BigInt() throws surface as misleading IdentityTxFailed.
4. Read paths silently swallow network/RPC errors as score: 0.
5. URL-typed fields accept javascript: / data: / file://.
6. feedbackHash cast to 0x${string} without hex validation.

[ckhbtc]

Blocking — optimistic tx-hash return on all write paths.

register, update (L258), deregister (L267), giveFeedback (L290), and revokeFeedback (L302) all return whatever txHash/txHashes the SDK hands back, with no on-chain confirmation. This is the same class of bug that PR #10 fixed for bridge/close flows.

agent_deregister is particularly risky — the tool is documented as "permanently burn an NFT" but the returned txHash could be from a broadcast that was never included. An LLM caller that sees { agentId, txHash } will treat the burn as complete; if the tx is actually dropped, a subsequent agent_register could collide, or the user could believe the identity is gone when it isn't.

Suggested fix: after each write, either poll client.getStatus(id) (or confirm the NFT is gone, for deregister) before returning, or wait for the receipt and check status === 'success' + the expected event topic (the repo already has fix(identity): add event topic check to giveFeedback log extraction in history, so the pattern is available). Accepting unconfirmed SDK results as success is inconsistent with the project's stance from #10.

[ckhbtc]

Blocking — confirm: boolean is LLM-controlled and not a meaningful guard.

The confirm flag is supplied by the same LLM context that decided to call agent_deregister in the first place. An adversarial or hallucinating prompt can trivially pass confirm: true; there is no out-of-band human confirmation, no ownership check, and no second factor before the SDK call.

Other destructive tools in this repo (bridge withdrawals, position close) don't take a runtime confirm parameter — they rely on explicit user phrasing + a strong tool description. A runtime confirm: boolean forwarded from the LLM looks like a safety barrier but is weaker than it appears.

Suggested fix: at minimum, do a server-side ownership check before calling client.deregister — fetch status, verify params.address (or the signer derived from it) is the actual NFT owner, and throw a typed error if not. That turns the confirm guard into defense-in-depth rather than the only line of defense. Consider also removing the confirm param entirely and letting tool description + signer-ownership do the work, matching the rest of the codebase.

[ckhbtc]

agentId: z.string().min(1) accepts any non-empty string, but downstream every use is BigInt(params.agentId). Values like "abc", "12n", "1.5" pass Zod validation then throw SyntaxError inside the try block in src/identity/index.ts, which wrapSdkError re-throws as IdentityTxFailed: Cannot convert abc to a BigInt — a misleading "tx failed" error for what is really input validation.

Same issue on every agentId schema in this file (agent_update, agent_deregister, agent_status, agent_list, agent_reputation, agent_feedback_list, agent_give_feedback, agent_revoke_feedback).

Fix: z.string().regex(/^\d+$/, 'agentId must be a non-negative integer string'). Same treatment for feedbackIndex, value (if string-typed), etc.

[ckhbtc]

reputation and feedbackList (L158) both swallow _err unconditionally and return zeros / empty arrays. This masks network outages, RPC errors, and contract reverts as "no data" — an LLM consumer gets { score: 0, count: 0 } even when the registry is temporarily unreachable.

When an agent-to-agent trust decision (or a human deciding whether to trust an agent) is based on reputation, the difference between "this agent has no reputation" and "we couldn't reach the chain" is load-bearing.

Suggested fix: narrow the catch to only handle the specific not-found / empty-results case (e.g., check error type or message), and re-throw network/RPC errors so the caller can distinguish. If the SDK doesn't surface a typed "not found" error, consider a try { getStatus } catch { IdentityNotFound } pre-check and let other errors propagate.

[ckhbtc]

URL-typed fields are plain z.string().optional() — image (L877), uri (L881, L918), sourceCode (L886, L925), documentation (L887, L926), and the update-side image (L914) all pass javascript:, data:, file://, or any other URI scheme. These strings flow unmodified into IPFS upload and end up in a persistent on-chain record funded by the user's gas.

The descriptions say "https://, http://, or ipfs://" but that's not enforced. The service-entry schema already uses z.string().url() correctly for endpoint, so the pattern exists in this file.

Fix: z.string().url() on each. If ipfs:// needs to be accepted (URL parser rejects it in some Node versions), add an explicit allowlist regex: z.string().regex(/^(https?|ipfs):\/\//).

[ckhbtc]

feedbackHash is z.string().optional() but is cast to `0x${string}` in src/identity/index.ts:282 with no validation. An LLM passing arbitrary text gets a silent type cast and then a cryptic SDK-level failure when the contract rejects a non-32-byte value.

Fix: z.string().regex(/^0x[0-9a-fA-F]{64}$/, 'feedbackHash must be a 32-byte hex string').optional().