Conversation
Ai bankofai patch 1
Code Review ReportProject: x402-tron / bankofai docs PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR adds a substantial new documentation section covering the Agent-Wallet product — a local encrypted signing tool for AI agents operating on TRON and EVM-compatible blockchains. The additions include a full English and Chinese documentation suite: an introduction, quick-start guide, CLI reference, SDK guide with TypeScript/Python examples, and an FAQ. Existing MCP Server documentation (TRON and SUN) is refactored to link out to the new Agent-Wallet docs rather than duplicating setup instructions — a clear improvement in information architecture. The CI workflow is updated for this branch and the package version is bumped to Overall the documentation quality is high: content is well-structured, security trade-offs are explained honestly, and the dual-language parity (EN + zh-Hans) is consistently maintained. Two substantive security concerns require fixes before merge: the QuickStart password-injection guide inadvertently stores the master password in shell history, and the CI debug step leaks the Docker Hub username into public workflow logs. Several minor issues around code-example robustness, link targets, and commit-message hygiene are also noted. Change Summary1. New Agent-Wallet Documentation (English)
Purpose: Introduces a fully self-contained developer documentation hub for Agent-Wallet. 2. New Agent-Wallet Documentation (Chinese / zh-Hans)
Purpose: Full Chinese-language parity for all new Agent-Wallet documentation. 3. MCP Server Documentation Refactor
Purpose: Eliminate documentation duplication; centralize Agent-Wallet knowledge; correct password-quoting bug (double-quotes → single-quotes for 4. Configuration & CI Changes
Detailed FindingsMajor[MJ-01] Shell History Exposes Master Password When Following QuickStart Instructions
Description The QuickStart guide (English and Chinese) instructs users to persist their master password by running: echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.zshrcThis The irony is significant: the documentation warns about environment variable scanning and AI assistants reading project directories, yet the recommended setup procedure places the password in one of the most commonly-read plaintext files on the system. Code # QuickStart.md - Step 2, Mac tab
echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.zshrc
# QuickStart.md - Step 2, Windows/Linux tab
echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.bashrcRecommendation Replace the # Option A: suppress this command from history (prepend a space — works in zsh/bash with HISTCONTROL=ignorespace)
echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.zshrc
# Option B: edit the file directly (no history exposure)
# Open ~/.zshrc in your editor and add the line manually:
# export AGENT_WALLET_PASSWORD='your-master-password'Add a caution admonition: :::caution This command will appear in your shell history
After running the command above, optionally delete it from history:
- **zsh**: `history -d $(history 1 | awk '{print $1}')`
- **bash**: `history -d $(history | tail -1 | awk '{print $1}')`
Or edit `~/.zshrc` / `~/.bashrc` directly with a text editor to avoid the issue entirely.
:::[MJ-02] CI Debug Step Leaks Docker Hub Username to Public Workflow Logs
Description The updated workflow retains a debug step that prints the Docker Hub username directly to GitHub Actions log output: - name: Debug Docker Hub secrets
if: github.event_name != 'pull_request'
...
run: |
echo "username=[$DOCKERHUB_USERNAME]"
echo "token_length=${#DOCKERHUB_TOKEN}"This step runs on every push to The token length ( Code - name: Debug Docker Hub secrets
if: github.event_name != 'pull_request'
env:
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
run: |
echo "username=[$DOCKERHUB_USERNAME]" # ← exposes account name in logs
echo "token_length=${#DOCKERHUB_TOKEN}"Recommendation Remove this debug step entirely before merging. It was likely useful during initial setup but has no place in a merged workflow: # DELETE the entire "Debug Docker Hub secrets" stepIf debugging is needed in future, use GitHub's secret masking feature and avoid echoing any secret-derived values. Minor[MN-01] TypeScript EVM Example Uses Non-Null Assertion on Potentially Null
|
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness |
| File | docs/Agent-Wallet/Developer/SDK-Cookbook.md : Lines 539–551; Chinese equivalent |
Description
The BSC/EVM transfer example uses the TypeScript non-null assertion operator (!) on feeData.maxFeePerGas and feeData.maxPriorityFeePerGas. These fields are typed as bigint | null in ethers.js because not all chains support EIP-1559. On legacy chains (or chains where the RPC doesn't advertise EIP-1559 fee data), these will be null, and the ! assertion will silently pass TypeScript's type checker while causing a runtime crash when the unsigned transaction is constructed.
Since the documentation is explicitly aimed at developers copying these examples into production environments, the example should model correct null-handling.
Code
const feeData = await rpcProvider.getFeeData();
const unsignedTx = {
...
maxFeePerGas: feeData.maxFeePerGas!, // ← could be null → runtime crash
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas!, // ← same issue
...
};Recommendation
Add an explicit null-check before building the transaction:
const feeData = await rpcProvider.getFeeData();
if (!feeData.maxFeePerGas || !feeData.maxPriorityFeePerGas) {
throw new Error("Chain does not support EIP-1559 fee data");
}
const unsignedTx = {
...
maxFeePerGas: feeData.maxFeePerGas,
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas,
...
};[MN-02] bankofai-recharge Table Link Points to a Live MCP Endpoint, Not a Documentation Page
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / UX |
| File | docs/Openclaw-extension/Intro.md : Line 2106; i18n/zh-Hans/.../Openclaw-extension/Intro.md : Line 4206 |
Description
The PR converts the plain text bankofai-recharge entry in the MCP server table into a hyperlink:
| **[bankofai-recharge](https://recharge.bankofai.io/mcp)** | BANK OF AI (Remote) | ... |The target URL (https://recharge.bankofai.io/mcp) is the live MCP SSE/HTTP endpoint itself, not a documentation or landing page. Clicking this link in a browser will present raw Server-Sent Events or JSON (depending on the server implementation), which is confusing for documentation readers. The other two entries in the same table link to GitHub repository pages — human-readable pages with READMEs.
Recommendation
Link to a documentation page or GitHub repo instead of the raw endpoint:
| **[bankofai-recharge](https://github.com/BofAI/bankofai-recharge)** | BANK OF AI (Remote) | Remote recharge MCP — top up your BANK OF AI account via on-chain USDT. Default endpoint: `https://recharge.bankofai.io/mcp` |If no documentation page exists yet, keep the name unlinked (as it was before) and only show the endpoint URL in the description column.
[MN-03] Feature Branch Trigger Retained in Docker CI Workflow May Persist After Merge
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Compatibility / CI |
| File | .github/workflows/docker.yml : Lines 6–18 |
Description
The PR changes the Docker CI push/pull_request trigger from ai-bankofai-patch-1 to update-mcp-server. Both are feature branch names. After this PR is merged to main, the main branch copy of docker.yml will contain update-mcp-server as a branch trigger. Pushes to update-mcp-server will then continue to trigger CI builds even after the PR is merged and the branch is no longer active. This is a minor operational annoyance and could lead to unexpected image builds if the branch is reused.
Code
on:
push:
branches:
- main
- master
- update-mcp-server # ← feature branch name, will remain after merge
pull_request:
branches:
- main
- master
- update-mcp-server # ← sameRecommendation
Remove the feature branch entry before merging, restoring the workflow to the standard main / master triggers (or, if there are legitimate reasons to keep feature branch CI, document them and use a pattern like feature/**):
on:
push:
branches:
- main
- master
pull_request:
branches:
- main
- master
workflow_dispatch:[MN-04] Inconsistent Commit Message Quality Makes Change History Hard to Navigate
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality / Maintainability |
| File | Git history |
Description
The majority of commits in this PR use non-descriptive messages: fix, fiix, fxi, update, update agent-wallet. With 19 commits introducing ~3,800 lines of documentation changes, the git history provides no meaningful information about what was changed or why. This makes bisecting, reverting, and reviewing individual changes significantly harder, especially for a documentation project where the "what changed" information is the primary value of the commit log.
Commit examples:
a956313 update
822bdae fix
224d32d fiix ← typo
3b87508 fix
47b8030 fxi ← typo
Recommendation
Consider squashing the PR into logical commits with descriptive messages before merge:
docs: add Agent-Wallet documentation (EN + zh-Hans)docs: refactor MCP Server docs to reference Agent-Walletci: update docker.yml branch trigger for update-mcp-serverfeat: bump version to 1.2.4 and update sidebar navigation
Suggestions
[S-01] Add Guidance to Prevent Shell History Exposure via HISTIGNORE
File: docs/Agent-Wallet/QuickStart.md, docs/Agent-Wallet/Developer/CLI-Reference.md
Description: The CLI Reference documents export AGENT_WALLET_PASSWORD='...' as the recommended setup pattern. Adding a note about using HISTIGNORE or HISTCONTROL=ignorespace (space-prefixed commands are not saved to history in most shells) would help security-conscious users avoid history exposure without changing the core UX flow.
Suggestion: Add a tip admonition: "To avoid recording this command in shell history, prefix it with a single space (requires HISTCONTROL=ignorespace or HISTIGNORE='*' in your shell config), or edit the rc file directly in a text editor."
[S-02] Document File Permissions for runtime_secrets.json
File: docs/Agent-Wallet/Developer/CLI-Reference.md : Lines 162–168 (Method B: Local Password Cache)
Description: The --save-runtime-secrets flag caches the master password in ~/.agent-wallet/runtime_secrets.json. The documentation does not mention that this file's permissions should be restricted (ideally 600 / user-read-only). If the ~/.agent-wallet/ directory is world-readable (e.g., on misconfigured multi-user systems), the cached password would be accessible to other local users — undermining the two-factor protection the tool is designed to provide.
Suggestion: Add a security note: "The cached password file at ~/.agent-wallet/runtime_secrets.json should have restrictive permissions (chmod 600). The tool sets this automatically on creation, but verify after any manual file operations."
[S-03] Consider Adding GitHub Actions Secret Usage Example for CI/CD Workflows
File: docs/Agent-Wallet/Developer/CLI-Reference.md (Non-Interactive Execution section)
Description: The CLI Reference describes "Method A: Environment Variable Injection (Recommended for AI Agents / CI Pipelines)" and shows a bare export AGENT_WALLET_PASSWORD=... command. In a GitHub Actions or GitLab CI context, the correct pattern is to use repository secrets (${{ secrets.AGENT_WALLET_PASSWORD }}), not shell exports. A small CI-context example would help developers avoid accidentally hardcoding the password in workflow YAML files.
Suggestion: Add a collapsible "GitHub Actions example" block in the CI section of the CLI Reference showing the proper env: / secrets: pattern for workflow files.
Positive Observations
| Area | Observation |
|---|---|
| Security messaging | The Intro and FAQ clearly and honestly explain the threat model, limitations, and the "password alone ≠ stolen funds" design — rare for product documentation |
| Password quoting fix | Both TRON and SUN deployment docs correctly update from "<password>" (double-quotes, allows shell expansion) to '<password>' (single-quotes, prevents expansion) — a real security improvement |
| Bad advice removal | The old SUN MCP Server FAQ advised "Avoid special characters, use alphanumeric combination" for passwords — terrible security guidance. The new text removes this entirely |
| Documentation consolidation | Replacing duplicated wallet setup steps across TRON and SUN FAQ/deployment docs with links to the central Agent-Wallet docs is an excellent DRY improvement |
| Dual-language parity | All 6 new documentation files have complete, high-quality Chinese translations. The i18n sidebar and JSON translations are also kept in sync |
| Code example completeness | The SDK Cookbook provides truly end-to-end runnable scripts (build → sign → broadcast) for both TRON and EVM chains, in both TypeScript and Python — unusually thorough |
| Dangerous operations section | The CLI Reference explicitly marks reset and change-password with :::danger admonitions, matching the severity of those operations |
| Non-interactive documentation | The three-method non-interactive execution guide (env var / cache / inline) is well-categorized with appropriate warnings for each method's security implications |
| Error type hierarchy | The SDK Guide documents the full exception class hierarchy, enabling developers to write correct error-handling code |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 6 | 5 | 1 | 0 | TypeScript null assertion on feeData [MN-01] |
| Security | 8 | 6 | 2 | 0 | Shell history [MJ-01]; CI debug leak [MJ-02] |
| Performance | 7 | 0 | 0 | 7 | Documentation-only PR |
| Code Quality | 6 | 4 | 2 | 0 | Commit message quality [MN-04]; endpoint link [MN-02] |
| Testing | 7 | 0 | 0 | 7 | Documentation-only PR |
| Documentation | 6 | 5 | 1 | 0 | runtime_secrets.json permissions undocumented [S-02] |
| Compatibility | 5 | 4 | 1 | 0 | Feature branch CI trigger retained [MN-03] |
| Observability | 4 | 0 | 0 | 4 | Not applicable to documentation changes |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between main and update-mcp-server. Runtime behavior, integration testing, and deployment impact are not covered. SDK behavioral correctness (e.g., actual return formats of signTransaction) could not be verified without the SDK source code.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-21
PR Audit ReportDate: 2026-03-21 PR Overview
Commit Summary (most recent first)
Files Changed (Grouped)New Documentation — Agent-Wallet section (English):
New Documentation — Agent-Wallet section (Simplified Chinese i18n):
Modified Documentation — MCP Server references (English + Chinese i18n):
Navigation & Configuration:
CI/CD:
Package:
Change Summary1. New Agent-Wallet Documentation SuiteThe dominant change in this PR is the addition of a complete, multi-page documentation section for the
Both English and Chinese Simplified (zh-Hans) versions are provided in parallel. 2. MCP Server Docs — Agent-Wallet Cross-linkingExisting SUN MCP Server and TRON MCP Server documentation has been updated to:
3. Sidebar Navigation
4. CI/CD Branch Reference Update
5. Package Version Bump
6. Openclaw Intro Link Fix
Detailed FindingsSecurity FindingsFINDING-001 —
|
| Priority | Finding | Severity |
|---|---|---|
| Must fix | FINDING-001: --save-runtime-secrets missing security warning |
Major |
| Should fix | FINDING-005: "Agent Wallet" sidebar label not translated in zh-Hans | Minor |
| Should fix | FINDING-006: No guidance on clearing the password cache after rotation | Minor |
| Should fix | FINDING-008: CI workflow will have dead branch trigger after merge | Minor |
| Consider | FINDING-004: Python broadcast example lacks 0x prefix comment |
Minor |
| Consider | FINDING-007: x402 example missing error handling | Minor |
| Consider | FINDING-009: All commit messages are non-descriptive | Suggestion |
| Consider | FINDING-010: Placeholder password Abc12345! used throughout |
Suggestion |
| Consider | FINDING-011: Non-standard Docusaurus emoji shortcodes in comparison table | Suggestion |
The blocking issue is FINDING-001: a feature that stores the master password in plaintext on disk is described as "the ultimate convenience solution" without any security warning. Given that this documentation's primary purpose is to guide users away from insecure private key handling, shipping a plaintext password cache recommendation without caveats would undermine the very security model the docs are built around.
Code Review ReportProject: BofAI Docs ( PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR adds a new Agent-Wallet documentation section to the BofAI docs site — a significant content addition comprising six new Markdown files covering introduction, quick start, FAQ, CLI reference, SDK guide, and SDK cookbook. It also refactors existing MCP Server docs to link to the new Agent-Wallet pages rather than duplicating setup instructions, updates sidebar navigation in both English and Chinese locales, bumps the package version to 1.2.4, and updates a GitHub Actions workflow branch filter. The documentation is generally well-written, well-structured, and security-conscious — it consistently warns users about the dangers of plaintext private keys, recommends local vault mode, and uses correct shell quoting guidance. The refactored MCP Server docs are cleaner and benefit from the new centralized Agent-Wallet docs. However, two major documentation security issues require attention before merge: the Change Summary1. New Agent-Wallet Documentation (6 new files)
Purpose: Centralize Agent-Wallet documentation, replacing scattered inline setup instructions in MCP Server docs. 2. MCP Server Docs Refactoring (5 modified files)
Purpose: Remove content duplication; all wallet setup details now live in the Agent-Wallet section. 3. Cross-Linking and Minor Doc Fixes (2 modified files)
4. Navigation and Configuration
5. Chinese i18n Content (6 new files)New Chinese translations under Detailed FindingsMajor[MJ-01]
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security / Documentation |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Lines 107–122 |
Description
The CLI-Reference introduces --save-runtime-secrets as a "True 'Set and Forget'" convenience method, describing it as "the ultimate convenience solution" and explaining that the password is "permanently cached in a local file (~/.agent-wallet/runtime_secrets.json)." No warning is provided about the security implications of this approach.
This is a significant security documentation gap. The entire Agent-Wallet value proposition rests on a two-factor security model: (1) the encrypted vault file and (2) the unlock password are stored separately. The --save-runtime-secrets feature collapses this into a single-factor model: anyone with read access to ~/.agent-wallet/ now has both the encrypted vault AND the plaintext password in the same directory. This is substantially weaker than the advertised dual-lock security model, yet the documentation presents it as an equally valid option with no downside.
Compare to how Method C (-p flag inline) receives a prominent :::danger block warning about terminal history. --save-runtime-secrets stores the password to disk with no such warning.
Code
### Method B: Local Password Cache (True "Set and Forget")
The ultimate convenience solution. After running a command once with the
`--save-runtime-secrets` flag, the password is permanently cached in a
local file (`~/.agent-wallet/runtime_secrets.json`). The next time you run
any signing command, the system automatically reads from the cache. No need
for inline passwords or environment variables:
```bash
agent-wallet sign msg "Hello" -n tron -p "Abc12345!" --save-runtime-secrets
**Recommendation**
Add a `:::caution` or `:::danger` admonition explaining that this stores the password in the same directory as the encrypted vault, reducing the dual-lock security model to a single-lock. Recommend this method only for fully isolated machines or development environments, and advise users to verify the file permissions of `runtime_secrets.json`:
```markdown
:::danger Security Trade-off: Collapses Dual-Lock to Single-Lock
`runtime_secrets.json` is stored **in the same directory as your encrypted
wallet file** (`~/.agent-wallet/`). This means anyone who can read that
directory now has both the password AND the encrypted keystore in one place —
the "two separate locks" security model no longer applies.
Only use this method on machines where the `~/.agent-wallet/` directory is
inaccessible to other users or processes. On shared or multi-tenant systems,
prefer Method A (environment variable injection) instead.
:::
[MJ-02] echo Command in QuickStart Silently Corrupts Passwords Containing Single Quotes
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness / Documentation |
| File | docs/Agent-Wallet/QuickStart.md : Lines 84–110 |
Description
The QuickStart guide instructs users to persist their master password with:
echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.zshrcThe surrounding :::caution block correctly warns about $ and ! characters, instructing users to keep the single quotes. However, the documentation does not address the case where the master password itself contains a single-quote character (').
If a user's auto-generated password is, for example, P@ss'w0rd!, the resulting shell config line would be:
export AGENT_WALLET_PASSWORD='P@ss'w0rd!''This is a shell syntax error. In some shells (e.g., bash) the variable would be silently set to P@ss (truncated at the first '), causing all subsequent signing commands to fail with an incorrect-password error — potentially leading users to believe their wallet is corrupted and reinitialize unnecessarily, losing access to funds if they have no mnemonic backup.
Code
**Step 1:** Copy the command below, replace the content inside the single quotes
with your actual password, paste it into the terminal and press Enter:
```bash
echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.zshrc:::caution Password has special characters? Don't touch the single quotes!
...
✅ Correct — single quotes, password saved as-is
echo "export AGENT_WALLET_PASSWORD='P@ss$w0rd!'" >> ~/.zshrc
**Recommendation**
Either:
1. **Verify that the `agent-wallet start` password generator never produces passwords containing single quotes** (acceptable if true, but should be documented), and add a note: *"Note: if you set your own password manually, avoid using single-quote characters."*
2. **Or document a `printf`-safe alternative** for arbitrary passwords:
```bash
# Handles any character including single quotes safely
printf "export AGENT_WALLET_PASSWORD='%s'\n" "$(printf '%s' 'your-master-password' | sed "s/'/'\\\\''/g")" >> ~/.zshrc
Option 1 is simpler and consistent with the beginner-friendly tone of the QuickStart page.
Minor
[MN-01] "Agent Wallet" Sidebar Category Missing Chinese Translation
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / i18n |
| File | i18n/zh-Hans/docusaurus-plugin-content-docs/current/sidebars.js : Lines 149–169 |
Description
The newly added "Agent Wallet" sidebar category label is left in English in the Chinese-locale sidebar (i18n/zh-Hans/.../sidebars.js). All other top-level categories in the same file have been translated (e.g., 'Openclaw 扩展插件', 'MCP Server' with Chinese subcategories). The nested subcategory "Explore Further" is correctly translated to '进一步探索', making the untranslated parent label an inconsistency.
Code
// i18n/zh-Hans/.../sidebars.js
{
type: 'category',
label: 'Agent Wallet', // <-- English label in Chinese locale
collapsed: false,
items: [
'Agent-Wallet/Intro',
'Agent-Wallet/QuickStart',
{
type: 'category',
label: '进一步探索', // <-- Correctly translated
...
},
'Agent-Wallet/FAQ',
],
},Recommendation
Either add a Chinese label:
label: 'Agent 钱包',Or, if "Agent Wallet" is intentionally kept as a brand name, add a comment noting the decision:
label: 'Agent Wallet', // Kept as brand name, intentionally untranslated[MN-02] Docker Workflow Triggers Build/Push on Every Push to Feature Branch
| Property | Value |
|---|---|
| Severity | Minor |
| Category | CI/CD |
| File | .github/workflows/docker.yml : Lines 5–16 |
Description
The Docker workflow now includes update-mcp-server in its push trigger list. As a result, every commit pushed to this feature branch (21 commits in this PR) triggers a Docker image build and push to Docker Hub with the test tag, overwriting the same tag repeatedly. This consumes unnecessary CI minutes, creates noisy image history, and risks pushing unstable documentation states to the test Docker tag.
The PR is still open, meaning this branch is likely to receive more commits before merge — each one will trigger another Docker build.
Code
on:
push:
branches:
- main
- master
- update-mcp-server # <-- feature branch triggers build + push
pull_request:
branches:
- main
- master
- update-mcp-serverRecommendation
Remove update-mcp-server from the push trigger. The pull_request trigger (which runs on PR events but skips the push step) is sufficient for CI validation during development:
on:
push:
branches:
- main
- master
pull_request:
branches:
- main
- master
- update-mcp-server
workflow_dispatch:[MN-03] Inconsistent Commit Messages Obscure Change History
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality / Maintainability |
| File | Commit history |
Description
The majority of commits on this branch use non-descriptive messages (fix, fiix, fxi, update, update agent-wallet). With 21 commits and only a handful carrying meaningful messages, the branch history does not convey what was actually changed in each commit. This makes it difficult to bisect regressions, understand the evolution of the documentation, or identify which commit introduced a specific change without diffing each one individually.
Recommendation
Before merge, consider squashing or rebasing the branch into logical commits with descriptive messages, for example:
docs: add Agent-Wallet introduction and quick start guidedocs: add Agent-Wallet developer docs (CLI reference, SDK guide, cookbook)docs: refactor MCP server docs to link to Agent-Wallet sectionconfig: add Agent-Wallet sidebar navigation and i18n labelsci: update docker workflow branch filter to update-mcp-serverchore: bump version to 1.2.4
Suggestions
[S-01] package.json Description Does Not Reflect Expanded Project Scope
File: package.json
Description: The description field still reads "x402-tron documentation", but the repository now covers Agent-Wallet, MCP Servers (TRON and SUN), Skills, and the Openclaw Extension. The narrow description may be misleading to contributors and automated tooling.
Suggestion: Update to something broader: "BofAI ecosystem documentation — Agent-Wallet, MCP Servers, Skills, and Openclaw Extension".
[S-02] AGENT_WALLET_DIR Environment Variable Example Uses Potentially Non-Expanding Tilde
File: docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md, docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md
Description: The example:
export AGENT_WALLET_DIR="~/.agent-wallet"places a tilde inside double quotes. In bash and zsh, tilde expansion does not occur inside double quotes, so this would literally set AGENT_WALLET_DIR to the string ~/.agent-wallet rather than the expanded /home/username/.agent-wallet. Whether the Agent-Wallet tool handles tilde expansion internally is not documented. This may cause unexpected directory not found errors on some systems.
Suggestion: Use $HOME for reliable expansion across all shells:
export AGENT_WALLET_DIR="$HOME/.agent-wallet"[S-03] --save-runtime-secrets Flow Does Not Show File Location Verification
File: docs/Agent-Wallet/Developer/CLI-Reference.md : Lines 107–122
Description: The --save-runtime-secrets feature stores credentials to ~/.agent-wallet/runtime_secrets.json but the documentation does not mention how to verify the file was created, what its structure looks like, or how to revoke/delete the cached secret if needed.
Suggestion: Add a note about how to list and remove cached secrets:
# View cached secrets (check what's stored)
cat ~/.agent-wallet/runtime_secrets.json
# Revoke cached secrets (forces password prompt on next run)
rm ~/.agent-wallet/runtime_secrets.json[S-04] SDK-Cookbook EVM Example Uses Non-Null Assertion Without Error Handling
File: docs/Agent-Wallet/Developer/SDK-Cookbook.md : Lines ~270–285
Description: The TypeScript EVM transfer example contains:
maxFeePerGas: feeData.maxFeePerGas!,
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas!,The ! non-null assertions suppress TypeScript's safety check. On legacy networks or certain RPC providers, feeData.maxFeePerGas may be null (EIP-1559 not supported), causing a runtime crash. Since this is a "copy and run" cookbook example, users may deploy this pattern in production without realizing the risk.
Suggestion: Add a null check with a clear error message:
if (!feeData.maxFeePerGas || !feeData.maxPriorityFeePerGas) {
throw new Error("Network does not support EIP-1559. Use a legacy gas price instead.");
}
const unsignedTx = {
...
maxFeePerGas: feeData.maxFeePerGas,
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas,
...
};Positive Observations
| Area | Observation |
|---|---|
| Security messaging | Consistent, well-calibrated security warnings throughout all new docs — the :::danger and :::caution admonitions are placed appropriately and the dual-lock security model is clearly explained in multiple places |
| Shell quoting guidance | Correct and prominent single-quote guidance for passwords with special characters ($, !) — a common footgun that the docs handle well |
| Non-interactive mode coverage | CLI-Reference covers all three non-interactive patterns (env var, cached secret, inline -p) with clear use-case guidance for AI agents and CI pipelines |
| Code example quality | SDK-Cookbook examples are complete end-to-end scripts with real API endpoints, error handling, and testnet-first defaults — users can genuinely copy-paste and run them |
| MCP docs refactoring | Removing duplicated wallet-setup code from MCP Server docs and replacing it with links is the right architectural decision — single source of truth reduces drift risk |
| i18n completeness | All six new English docs have corresponding Chinese translations, and the new i18n sidebar key is added to current.json correctly |
| Security quote fix | LocalPrivatizedDeployment.md (both SUN and TRON) corrects export AGENT_WALLET_PASSWORD="..." (double quotes) to export AGENT_WALLET_PASSWORD='...' (single quotes) — a concrete security improvement |
| Cross-linking | The new docs are well cross-linked: each page in the Agent-Wallet section ends with a "Next Steps" table pointing to related pages |
| Docusaurus integration | New sidebar correctly uses the collapsed: false / collapsed: true pattern for progressive disclosure — high-value pages (Intro, QuickStart, FAQ) are always visible; developer docs are nested under "Explore Further" |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 6 | 5 | 1 | 0 | MJ-02: echo command with single-quote passwords |
| Security | 8 | 7 | 1 | 0 | MJ-01: --save-runtime-secrets lacks security warning |
| Performance | 7 | 0 | 0 | 7 | Documentation-only PR, not applicable |
| Code Quality | 6 | 3 | 2 | 1 | MN-03: commit messages; S-04: non-null assertions in examples |
| Testing | 6 | 0 | 0 | 6 | Documentation-only PR, not applicable |
| Documentation | 6 | 4 | 2 | 0 | MN-01: missing i18n translation; S-02: tilde expansion |
| Compatibility | 5 | 4 | 0 | 1 | |
| Observability | 4 | 0 | 0 | 4 | Documentation-only PR, not applicable |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between main and update-mcp-server. Runtime behavior, integration testing (e.g., actual Docusaurus build validation, broken link checking), and deployment impact are not covered. Actual behavior of the --save-runtime-secrets feature and the Agent-Wallet password generator's character set were inferred from documentation alone and should be verified against the implementation.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-22
Code Review ReportProject: x402-tron/docs PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR introduces a comprehensive new Agent Wallet documentation section (6 new pages in English + 6 Chinese translations) and updates existing MCP Server documentation to cross-reference the new material. The documentation covers installation, CLI usage, SDK integration (TypeScript & Python), and full end-to-end code examples including real blockchain transactions on TRON and BSC. The overall quality of the documentation content is high — it is well-structured, includes appropriate security warnings, and provides useful bilingual coverage. The security guidance is sound: the docs correctly recommend encrypted local vault mode over plaintext private key exposure, explain the two-lock security model, and warn readers explicitly about the risks of static injection mode and plaintext password caching. Two Major issues were found in the SDK Cookbook's TypeScript code examples that developers are likely to copy verbatim into production: a non-null assertion on Change Summary1. New Agent Wallet Documentation (English)
Purpose: Introduce a new first-class documentation section for the 2. New Agent Wallet Documentation (Chinese / i18n)
Purpose: Provide full bilingual coverage for the new documentation. 3. Sidebar Navigation Updates
Purpose: Make the new Agent Wallet section discoverable in the left-hand documentation navigation. 4. MCP Server Documentation Updates
Purpose: DRY up Agent Wallet setup instructions across all MCP Server pages, pointing to the new canonical docs; fix the incorrect double-quote password variable pattern. 5. Configuration & CI Changes
Purpose: Keep the Docker CI workflow aligned with the active development branch; increment package version. Detailed FindingsMajor[MJ-01] Non-null Assertions on
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness / Code Quality |
| File | docs/Agent-Wallet/Developer/SDK-Cookbook.md : Lines 543–555 |
Description
The TypeScript BSC transfer example uses the non-null assertion operator (
!) onfeeData.maxFeePerGasandfeeData.maxPriorityFeePerGas. These fields are typed asbigint | nullin ethers.js v6 — they returnnullwhen queried on non-EIP-1559 networks or when legacy gas price mode is in effect. Using!suppresses the TypeScript compiler error but causes a silentnullto be passed into the transaction object, which will result in a transaction submission failure or0gas being applied on certain chains.Since this is example code that developers will copy directly into production, demonstrating unsafe non-null assertions sets a poor precedent and may lead to bugs in user code.
Code
// docs/Agent-Wallet/Developer/SDK-Cookbook.md
const feeData = await rpcProvider.getFeeData();
const unsignedTx = {
to: toAddress,
value: ethers.parseEther(amountEther),
gas: 21000n,
maxFeePerGas: feeData.maxFeePerGas!, // ← null assertion
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas!, // ← null assertion
nonce,
chainId: CHAIN_ID,
type: 2, // EIP-1559
};Recommendation
const feeData = await rpcProvider.getFeeData();
if (!feeData.maxFeePerGas || !feeData.maxPriorityFeePerGas) {
throw new Error(
"Network does not support EIP-1559 fee data. " +
"Try switching to a legacy gas price transaction (type: 0)."
);
}
const unsignedTx = {
to: toAddress,
value: ethers.parseEther(amountEther),
gas: 21000n,
maxFeePerGas: feeData.maxFeePerGas,
maxPriorityFeePerGas: feeData.maxPriorityFeePerGas,
nonce,
chainId: CHAIN_ID,
type: 2,
};[MJ-02] Stale agent-wallet init → agent-wallet start Transition Not Explicitly Noted
| Property | Value |
|---|---|
| Severity | Major |
| Category | Documentation / Correctness |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md : Lines 91–99 |
Description
The old TRON MCP Server FAQ section contained an explicit
agent-wallet initcommand as the re-initialization command (removed in this PR). The new Agent Wallet documentation consistently usesagent-wallet startfor wallet initialization. The deletion of the old block is appropriate, but users who previously bookmarked or cached the old instructions and then search foragent-wallet initwill find no documentation about the command change.More critically, the
TRONMCPServer/FAQ.mdpreviously had a clear code block showingagent-wallet initas the recovery path. The replacement just says "see the Agent-Wallet Quick Start" — but the new QuickStart only showsagent-wallet startfor new wallets, not specifically for the "reset + re-initialize after password loss" scenario. This leaves a gap in the troubleshooting story.
Code
-2. **If the password is lost**, you'll need to re-initialize. Note: this creates a new wallet — funds in the old wallet must be recovered through other means.
- ```bash
- agent-wallet init
- ```
+If the password is lost, you'll need to re-initialize — see the [Agent-Wallet Quick Start](../../../Agent-Wallet/QuickStart) and [Agent-Wallet FAQ](../../../Agent-Wallet/FAQ) for details.Recommendation
Add an explicit note that agent-wallet init was renamed to agent-wallet start, or add a "Reset & Re-initialize" section to docs/Agent-Wallet/FAQ.md that explicitly documents the recovery workflow with the current agent-wallet reset + agent-wallet start sequence. The FAQ.md mentions agent-wallet reset but could be more explicit for the "after losing your password" scenario.
If the password is lost, you'll need to re-initialize your wallet using
`agent-wallet reset` followed by `agent-wallet start` — see [Agent-Wallet FAQ](../../../Agent-Wallet/FAQ#-what-if-i-forget-my-password)
for details. **Note**: This permanently deletes the old wallet; ensure you have
a mnemonic backup before proceeding.Minor
[MN-01] CI Workflow Branch Trigger Will Be Stale After Merge
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Compatibility / Maintainability |
| File | .github/workflows/docker.yml : Lines 9–18 |
Description
The Docker build workflow now triggers on
pushandpull_requestevents targeting theupdate-mcp-serverbranch. Once this PR is merged intomain, theupdate-mcp-serverbranch will no longer be the active development branch. The workflow trigger will remain forever, triggering builds for a branch that may never receive another push, or worse, may be reused for an unrelated purpose in the future.
Code
on:
push:
branches:
- main
- master
- update-mcp-server # ← becomes stale post-merge
pull_request:
branches:
- main
- master
- update-mcp-server # ← becomes stale post-mergeRecommendation
Remove the update-mcp-server branch trigger as a follow-up after this PR is merged, or use a wildcard pattern to capture all feature branches:
on:
push:
branches:
- main
- master
pull_request:
branches:
- main
- master
workflow_dispatch:[MN-02] Python Cookbook Examples Do Not Check HTTP Response Status Codes
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness / Code Quality |
| File | docs/Agent-Wallet/Developer/SDK-Cookbook.md : Lines 420–453 |
Description
The Python TRON transfer example uses
aiohttpto call TronGrid endpoints but does not checkresp.statusbefore attempting to parse the JSON body. If TronGrid returns a 4xx or 5xx error,await resp.json()will succeed (parsing the error JSON body), but the code will fall through to thetxIDcheck without signaling the HTTP error condition clearly. This is a teachable moment that the docs miss, and developers copying this code may find silent failures in error scenarios.
Code
async with session.post(
f"{TRONGRID_API}/wallet/createtransaction",
json={...},
) as resp:
unsigned_tx = await resp.json() # no status check
if "txID" not in unsigned_tx:
raise ValueError(f"Failed to build transaction: {unsigned_tx}")Recommendation
async with session.post(
f"{TRONGRID_API}/wallet/createtransaction",
json={...},
) as resp:
resp.raise_for_status() # raises ClientResponseError on 4xx/5xx
unsigned_tx = await resp.json()
if "txID" not in unsigned_tx:
raise ValueError(f"Failed to build transaction: {unsigned_tx}")[MN-03] Missing Blank Line Before --- Separator in Chinese CLI Reference
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / Formatting |
| File | i18n/zh-Hans/docusaurus-plugin-content-docs/current/Agent-Wallet/Developer/CLI-Reference.md : Lines 2350–2352 |
Description
The Chinese CLI Reference is missing a blank line before the
---horizontal rule that closes the shell script example section. This may cause Docusaurus/MDX to render the---as a continuation of the preceding fenced code block instead of a thematic break, resulting in incorrect page rendering.
Code
# 接下来，你可以拿这个 $SIGNATURE 去发请求、拼 JSON，或者传给其他流水线任务...--- ← no blank line before this
**Recommendation**
```markdown
# 接下来，你可以拿这个 $SIGNATURE 去发请求、拼 JSON，或者传给其他流水线任务...
Note: The corresponding English file (`docs/Agent-Wallet/Developer/CLI-Reference.md`) correctly has a blank line before this separator (line 218 in the diff). The Chinese version should be updated to match.
---
### Suggestions
#### [S-01] SDK-Cookbook TypeScript: Verify `signedTxHex` Prefix Assumption is Documented
**File:** `docs/Agent-Wallet/Developer/SDK-Cookbook.md` : Lines 563–564
**Description:** The code calls `rpcProvider.broadcastTransaction("0x" + signedTxHex)`, which assumes `wallet.signTransaction()` returns a hex string *without* the `0x` prefix. While the SDK Guide does state this ("all signing methods return a hex-encoded signature string (without the `0x` prefix)"), this implicit contract is not re-stated in the Cookbook where the code is likely to be copied in isolation.
**Suggestion:** Add an inline comment to make this assumption explicit:
```typescript
// wallet.signTransaction() returns hex without the "0x" prefix
const txResponse = await rpcProvider.broadcastTransaction("0x" + signedTxHex);
[S-02] QuickStart.md Step 2: Echo Command May Double-Write if Already Configured
File: docs/Agent-Wallet/QuickStart.md : Lines 1816–1817
Description: The recommended command echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.zshrc will append the export line every time the user runs it. If a user follows the guide twice (e.g., after a password change), they will end up with duplicate entries in their shell config, with the older entry shadowing the newer one on some systems.
Suggestion: Either document this caveat, or recommend a pattern that checks for and replaces existing entries:
# Safer: replace existing entry if present, append if not
grep -q 'AGENT_WALLET_PASSWORD' ~/.zshrc \
&& sed -i "s|export AGENT_WALLET_PASSWORD=.*|export AGENT_WALLET_PASSWORD='your-master-password'|" ~/.zshrc \
|| echo "export AGENT_WALLET_PASSWORD='your-master-password'" >> ~/.zshrcAlternatively, simply add a note: "If you've already configured this, edit ~/.zshrc directly instead of running the echo command again."
[S-03] SDK-Guide.md: AGENT_WALLET_DIR Default Uses ~ Which May Not Expand in All Contexts
File: docs/Agent-Wallet/Developer/SDK-Guide.md : Line 980 (Environment Variable Reference table)
Description: The table shows the default for AGENT_WALLET_DIR as ~/.agent-wallet. However, ~ is a shell shorthand that does not expand in all programming contexts (e.g., Python's os.path.exists("~/.agent-wallet") would fail). The docs don't clarify that the SDK resolves ~ to the home directory.
Suggestion: Clarify that the SDK handles home directory resolution, or show the expansion explicitly: "$HOME/.agent-wallet".
[S-04] LocalPrivatizedDeployment.md: Leftover AGENT_WALLET_DIR Still Uses Double Quotes
File: docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md : Lines 2001–2003 (diff context)
Description: The PR correctly changes AGENT_WALLET_PASSWORD to use single quotes, but the accompanying AGENT_WALLET_DIR line in the same block still uses double quotes. While ~/.agent-wallet doesn't contain shell-special characters that would cause a problem, it's inconsistent with the now-established single-quote convention. The same applies to the TRON equivalent.
Code:
export AGENT_WALLET_PASSWORD='<your-master-password>'
# Optional: specify custom wallet directory (default: ~/.agent-wallet)
export AGENT_WALLET_DIR="~/.agent-wallet" # ← still double quotesSuggestion: Either use single quotes consistently, or document why double quotes are acceptable here (the path contains no special characters). Consistency reduces user confusion.
Positive Observations
| Area | Observation |
|---|---|
| Security Warnings | The documentation is exemplary in clearly marking security-sensitive operations with :::danger and :::caution admonitions. Every insecure pattern (inline -p, --save-runtime-secrets, static injection mode) is accompanied by a prominent, plaintext-language warning. |
| Single-quote Fix | The PR correctly fixes export AGENT_WALLET_PASSWORD="..." (double quotes) to export AGENT_WALLET_PASSWORD='...' (single quotes) in all modified deployment guides — a subtle but important security improvement that prevents shell expansion of special characters in passwords. |
| Bilingual Coverage | All 6 new English pages have corresponding Chinese (zh-Hans) translations, and the sidebar navigation and translation JSON are correctly synchronized. |
| Error Handling in SDK Examples | The SDK Guide's error handling section is thorough — it documents all error types, shows proper try/catch patterns in both TypeScript and Python, and includes the error type hierarchy. |
| Code Quality in Examples | Python examples correctly use asyncio with aiohttp async context managers, and properly await async calls throughout. TypeScript examples use ethers.js v6 idioms correctly (e.g., ethers.parseEther, ethers.JsonRpcProvider). |
| DRY Principle Applied | The refactoring of Agent Wallet setup instructions from inline blocks in each MCP Server page to a single canonical documentation section is a textbook application of the DRY principle, significantly reducing maintenance burden. |
| Security Model Documentation | The Intro.md threat model (environment variable scanning, dependency poisoning, git history leaks) is clear, technically accurate, and compelling. The two-lock security model explanation is well articulated. |
| Password Special Character Guidance | The consistent, repeated guidance about using single quotes for passwords containing $, !, and other shell-special characters — with explicit correct/incorrect examples — is excellent defensive documentation. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 6 | 4 | 2 | 0 | Non-null assertions (MJ-01); stale command reference (MJ-02) |
| Security | 8 | 8 | 0 | 0 | All sensitive operations properly warned |
| Performance | 7 | 0 | 0 | 7 | Documentation-only PR; not applicable |
| Code Quality | 8 | 6 | 2 | 0 | Non-null assertion pattern (MJ-01); missing HTTP status check (MN-02) |
| Testing | 5 | 0 | 0 | 5 | Documentation-only PR; not applicable |
| Documentation | 6 | 5 | 1 | 0 | Formatting issue in zh-Hans file (MN-03) |
| Compatibility | 4 | 3 | 1 | 0 | Stale CI branch trigger (MN-01) |
| Observability | 4 | 0 | 0 | 4 | Documentation-only PR; not applicable |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between main and update-mcp-server. Runtime behavior, integration testing, and deployment impact are not covered.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-22
Code Review ReportProject: x402-tron/docs PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR introduces the Agent Wallet documentation section — a new first-class product area covering installation, quick start, CLI reference, SDK guide, and SDK cookbook. In addition to the 6 new documentation files, the PR updates existing MCP Server docs to link to the new Agent Wallet section instead of repeating inline instructions, cleans up a security-sensitive debug step in the CI workflow, and bumps the package version. The i18n mirror (zh-Hans) is also kept in sync with all new and changed content. The overall quality of this PR is high. The new documentation is comprehensive, well-structured, and internally cross-linked. A notable security improvement is the removal of a CI step that was printing Docker Hub credentials to build logs. A Major finding is raised regarding an inherently insecure wallet type ( Change Summary1. CI/CD Workflow Update
Purpose: Update CI to track the new feature branch and remove a security anti-pattern (logging credential metadata). 2. Package Version Bump
Purpose: Reflect the documentation content additions in the package version. 3. Navigation / Sidebar Registration
Purpose: Surface the new Agent Wallet section in the site navigation. 4. New Agent Wallet Documentation (English + zh-Hans)
Purpose: Provide complete documentation for the Agent Wallet product across user levels (casual user → developer). 5. Updated MCP Server Documentation
Purpose: DRY the documentation — remove duplicate instructions and point readers to the canonical Agent Wallet section. Detailed FindingsMajor[MJ-01]
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security / Documentation |
| File | docs/Agent-Wallet/FAQ.md : Lines 66–77; docs/Agent-Wallet/Developer/SDK-Guide.md (static mode); docs/Agent-Wallet/Developer/SDK-Cookbook.md (Prerequisites section) |
Description
The raw_secret wallet type stores the private key in plaintext on disk. The documentation describes this in a comparison table with the caption "Always use local_secure", but then proceeds to document static-mode environment variables (AGENT_WALLET_PRIVATE_KEY) as a first-class feature throughout the SDK Guide and Cookbook prerequisites. The product's core value proposition is eliminating plaintext key exposure, yet raw_secret directly contradicts this — and it is documented without a :::danger callout or equivalent prominent warning.
A user following the SDK Cookbook prerequisites could inadvertently configure AGENT_WALLET_PRIVATE_KEY (static mode) without understanding they've recreated the exact threat model the product claims to solve.
Code
<!-- docs/Agent-Wallet/FAQ.md -->
| | `local_secure` | `raw_secret` |
| :--- | :---: | :---: |
| **Key encryption** | ✅ Strong encryption | ❌ Plaintext |
| **If an agent reads the file** | ✅ Key is inaccessible | ❌ Stolen instantly |
| **Use case** | ✅ All scenarios | ⚠️ Fully isolated dev environments only |
**Always use `local_secure`** unless you're 100% certain no other agent is running on that machine.<!-- docs/Agent-Wallet/Developer/SDK-Cookbook.md — Prerequisites section -->
3. Set `AGENT_WALLET_PASSWORD` (local `local_secure` mode) or `AGENT_WALLET_PRIVATE_KEY` (static mode)Recommendation
Add a :::danger admonition wherever raw_secret / static mode (AGENT_WALLET_PRIVATE_KEY) is mentioned. In the Cookbook prerequisites, either (a) remove the static-mode option entirely and direct readers to the Quick Start for wallet setup, or (b) wrap it in an explicit danger block:
:::danger Static mode exposes your private key as plaintext
`AGENT_WALLET_PRIVATE_KEY` stores your key unencrypted in an environment variable — the exact exposure `local_secure` mode is designed to prevent. Only use this in a fully isolated, offline test environment with no other processes running.
:::Minor
[MN-01] --save-runtime-secrets Flag Caches Master Password in Plaintext
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Documentation |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Method B section |
Description
The --save-runtime-secrets feature persists the master password in plaintext to ~/.agent-wallet/runtime_secrets.json. The documentation uses a :::danger callout, which is appropriate, but the feature is still presented as a valid "Method B" — a "Set and Forget" solution — alongside the safer environment variable approach. Given that the product's security model relies on the physical-file + password separation, caching the password on disk alongside the wallet file completely defeats the dual-lock model.
Code
### Method B: Local Password Cache (True "Set and Forget")
...
agent-wallet sign msg "Hello" -n tron -p "Abc12345!" --save-runtime-secrets
:::danger Security Warning
`runtime_secrets.json` stores your master password in **plaintext**. Any program with access
to your file system can read it directly.
:::Recommendation
Consider demoting this to an appendix or "Advanced / Dangerous" section rather than presenting it as a numbered method equivalent to the environment variable approach. Alternatively, add a note explicitly stating that this negates the dual-lock security model:
:::danger This disables the dual-lock protection
Caching the password next to the wallet file means a single file system compromise grants full access to funds — defeating the core security model of Agent-wallet. Use only for throwaway test wallets.
:::[MN-02] Inline -p Password Flag Documented Without Discouraging Shell History Exposure
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Documentation |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Method C section |
Description
Method C documents passing the master password inline via -p "Abc12345!". The accompanying :::danger callout correctly warns that the password will appear in shell history, but the same -p flag is also used without any warning in the agent-wallet start command examples at the top of the file (e.g., agent-wallet start -p Abc12345!). A user following the "Basic Commands" section first will not see the history warning until they reach Method C further down.
Code
<!-- Top of CLI-Reference.md — no warning here -->
**Custom password:**
```bash
agent-wallet start -p Abc12345!:::danger Security Warning
When using -p to pass the password inline, the plaintext password is permanently
recorded in your terminal's history.
:::
**Recommendation**
Add a `:::caution` note immediately after the first `agent-wallet start -p` example near the top of the file, pointing to the password configuration section:
```markdown
:::caution Shell history risk
Passing `-p` inline records the password in your terminal history. For production wallets,
prefer setting `AGENT_WALLET_PASSWORD` as an environment variable — see [Password Configuration](#password-configuration).
:::
[MN-03] update-mcp-server Branch Trigger Remains in CI After Merge
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Compatibility / CI |
| File | .github/workflows/docker.yml : Lines 8, 14 |
Description
The docker.yml workflow was updated to trigger on push and pull_request events for the update-mcp-server branch. This is appropriate while the PR is in development, but once this branch is merged into main, the update-mcp-server branch trigger becomes dead weight. More importantly, if another developer creates a branch named update-mcp-server in the future, it will inadvertently trigger CI builds and Docker Hub pushes.
Code
on:
push:
branches:
- main
- master
- update-mcp-server # <-- should be removed post-merge
pull_request:
branches:
- main
- master
- update-mcp-server # <-- should be removed post-mergeRecommendation
Remove the update-mcp-server branch from both push and pull_request triggers in a follow-up commit immediately after this PR is merged:
on:
push:
branches:
- main
- master
pull_request:
branches:
- main
- master
workflow_dispatch:[MN-04] Potentially Unresolved Cross-Link in QuickStart.md
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | docs/Agent-Wallet/QuickStart.md : Step 3 section |
Description
QuickStart.md references ../Openclaw-extension/QuickStart.md in the Step 3 callout. A docs/Openclaw-extension/QuickStart.md file was not included in this PR's diff, nor is it visible in the changed files stat. If this file does not exist on the main branch either, Docusaurus will generate a broken link warning at build time (or a 404 at runtime if warnings are suppressed).
Code
:::info Haven't installed OpenClaw Extension yet?
Head to [OpenClaw Extension Quick Start](../Openclaw-extension/QuickStart.md) first, then come back here.
:::Recommendation
Verify whether docs/Openclaw-extension/QuickStart.md exists on the merged main branch. If it does not exist, either:
- Replace the link with the existing
docs/Openclaw-extension/Intro.md, or - Create a stub
QuickStart.mdin a follow-up PR.
Run yarn build (or docusaurus build) locally before merge to surface any broken-link errors.
Suggestions
[S-01] Commit Messages Are Non-Descriptive
File: .github/ / all changed files
Description: All 23 commits in this branch use generic messages ("fix", "update", "fiix"). This makes it impossible to trace which commit introduced which change when debugging regressions or doing git bisect.
Suggestion: Establish a commit message convention (e.g., Conventional Commits: docs(agent-wallet): add CLI reference) and apply it going forward. The existing history cannot be changed without a rebase, but this is worth addressing in future branches.
[S-02] i18n English Locale current.json Not Updated
File: i18n/en/docusaurus-plugin-content-docs/current.json
Description: The zh-Hans locale current.json was updated to add the "Explore Further" → "进一步探索" label. The English locale current.json does not appear to have a corresponding "Explore Further" entry added. Depending on Docusaurus configuration, this may result in the "Explore Further" sidebar category rendering without a translated label in the English locale.
Suggestion: Verify whether the English current.json requires an explicit entry for the "Explore Further" label, and add it if so:
"sidebar.docsSidebar.category.Explore Further": {
"message": "Explore Further",
"description": "The label for category Explore Further in sidebar docsSidebar"
}[S-03] OpenClaw Extension QuickStart Link in Intro.md Points to .md Extension
File: docs/Agent-Wallet/Intro.md
Description: The cross-link [OpenClaw Extension](../Openclaw-extension/Intro.md) uses a .md file extension. In Docusaurus, internal links can use either the .md extension or the bare path. Consistency with the rest of the codebase is preferable.
Suggestion: Audit whether the project convention omits .md in cross-links (e.g., ./QuickStart vs ./QuickStart.md) and normalise the new Agent Wallet docs accordingly.
[S-04] bankofai-recharge Link in Openclaw-extension/Intro.md Points to MCP Endpoint URL Instead of Docs
File: docs/Openclaw-extension/Intro.md
Description: The bankofai-recharge entry was changed from plain text to a hyperlink pointing to https://recharge.bankofai.io/mcp — the live MCP endpoint URL, not a documentation page. Users clicking this link would receive a raw MCP API response, not a human-readable page.
Suggestion: Either link to a dedicated documentation page for the recharge MCP (if one exists or will be created), or revert to plain text / a link to the bankofai.io main site, and keep the endpoint URL only in the description column:
| **bankofai-recharge** | BANK OF AI (Remote) | Remote recharge MCP — top up your BANK OF AI account via on-chain USDT. Default endpoint: `https://recharge.bankofai.io/mcp` |Positive Observations
| Area | Observation |
|---|---|
| Security — CI | The removal of the Debug Docker Hub secrets step eliminates a long-standing security anti-pattern where the Docker Hub token length (and username) were printed to publicly viewable build logs on every non-PR push. |
| Security — Documentation | QuickStart.md includes an excellent :::tip Why not use the echo command? callout explaining that shell history files are scanned by backup utilities and AI assistants — directly relevant to the product's threat model. |
| Security — Documentation | The password single-quote guidance ('P@ss$w0rd!' vs "P@ss$w0rd!") throughout the changed MCP docs correctly prevents silent shell expansion of $ characters in passwords. This change was applied consistently across both SUN and TRON deployment docs. |
| Documentation — DRY | Replacing verbose inline Agent Wallet setup instructions in MCP docs with concise sentences + internal links is the correct approach. It reduces maintenance burden and ensures users always see up-to-date instructions. |
| Documentation — Structure | The new Agent Wallet section is well-layered: security pitch (Intro) → zero-friction onboarding (QuickStart) → full reference (CLI/SDK) → FAQs. The routing table at the bottom of each doc ("I want to… → Go here") is an excellent UX pattern for multi-audience documentation. |
| Documentation — Completeness | The SDK Cookbook provides complete, runnable end-to-end examples (TRON transfer, BSC transfer, x402 payment) including error handling, dependency installation, and expected output — significantly above the minimum bar for API documentation. |
| i18n | The zh-Hans locale is kept in sync with all new and modified English content in the same PR, avoiding documentation drift between languages. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 5 | 5 | 0 | 3 | No logic errors in config/sidebar changes; doc cross-links mostly correct (see MN-04) |
| Security | 8 | 6 | 2 | 0 | raw_secret exposure (MJ-01), plaintext password cache (MN-01); positive: debug secret step removed |
| Performance | 7 | 7 | 0 | 0 | Documentation-only changes; no performance impact |
| Code Quality | 8 | 6 | 0 | 2 | Commit message quality (S-01); internal link style inconsistency (S-03) flagged as suggestions |
| Testing | 5 | 2 | 0 | 3 | No automated link-checking present; docs build should be verified before merge (MN-04) |
| Documentation | 6 | 6 | 0 | 0 | Comprehensive; minor i18n gap (S-02) and broken external link (S-04) as suggestions |
| Compatibility | 5 | 4 | 1 | 0 | CI branch trigger should be cleaned up post-merge (MN-03) |
| Observability | 4 | 4 | 0 | 0 | N/A for docs changes; CI workflow change is observable |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between the specified branches (main...update-mcp-server). Runtime behavior, integration testing, and deployment impact are not covered.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-22
Code Review ReportProject: x402-tron/docs (BankOfAI Documentation) PR OverviewBranch Information
Commit History (Recent)
Review SummaryVerdict
Findings at a Glance
SummaryThis PR introduces a significant documentation expansion: a brand-new Agent Wallet section (six new files) covering introductory concepts, quick-start guidance, CLI reference, SDK guide, cookbook, and FAQ. It also substantially rewrites the SKILLS documentation for a more user-friendly, prompt-driven format, and fixes several pre-existing security issues in the MCP server documentation. The overall quality of this PR is high. The most impactful changes — removing Minor concerns centre on: a documented Change Summary1. CI/CD Workflow Update
Purpose: Replace the stale 2. Project Version Bump
Purpose: Increment the minor version to reflect this documentation release. 3. Sidebar Navigation Updates
Purpose: Wire up the new Agent Wallet documentation and the new SKILLS QuickStart page into the site navigation. 4. New Agent Wallet Documentation (6 new files)
Purpose: Introduce comprehensive documentation for the Agent Wallet product, covering installation, wallet management, signing operations, and SDK usage across TypeScript and Python. 5. MCP Server Documentation — Security & Accuracy Fixes
Purpose: Fix several pre-existing security issues in the MCP server docs; link new Agent Wallet documentation; modernise reset instructions. 6. SKILLS Documentation Rewrite
Purpose: Shift the SKILLS documentation from developer-oriented technical reference to user-first onboarding material; add an explicit quick-start path. Detailed FindingsMinor[MN-01]
|
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Documentation |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Method B section |
Description
The CLI Reference documents a --save-runtime-secrets flag that writes the master password to ~/.agent-wallet/runtime_secrets.json in plaintext. A :::danger admonition is present, but the feature is framed as a "convenience vs. security trade-off" — language that underplays the severity. For a product whose primary value proposition is the dual-lock security model, documenting a flag that silently tears down both locks creates a discoverability risk: a user who skims the heading without reading the warning may enable it on a real-funds wallet.
Code
### Method B: Local Password Cache (Convenience vs. Security Trade-off)
After running a command once with the `--save-runtime-secrets` flag, the password
is permanently cached in a local file (`~/.agent-wallet/runtime_secrets.json`).Recommendation
Replace the neutral framing ("Convenience vs. Security Trade-off") with an unambiguous scope restriction in the section heading and opening sentence, e.g.:
### Method B: Local Password Cache (Throwaway Test Wallets Only)
> **Do not use for wallets holding real funds.** This flag writes your master
> password to `~/.agent-wallet/runtime_secrets.json` in plaintext, permanently
> defeating Agent-wallet's dual-lock security model.Also consider adding --save-runtime-secrets to a "flags to avoid" table at the top of the CLI Reference so it surfaces before a user reaches the detailed description.
[MN-02] Inline -p Flag in CLI Examples Leaks Password to Shell History — Warning Is in the Wrong Section
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Documentation |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Method C section |
Description
The shell-history risk of passing passwords via -p is documented in Method A (environment variable section), but Method C ("Inline -p Flag") uses agent-wallet start -p Abc12345! as its primary example without repeating the warning. A reader who jumps directly to Method C — the most ergonomic option — will not encounter the caution.
Code
agent-wallet sign msg "Hello" -n eip155:56 -w my-bsc-wallet -p 'Abc12345!'Recommendation
Add a concise :::caution block at the top of the Method C section:
:::caution Shell history exposure
Passing `-p` inline records the password in your terminal's history file
(`.zsh_history` / `.bash_history`). Use this method for one-off commands only —
never in scripts, CI pipelines, or shared terminals.
:::[MN-03] workflow_dispatch Comment Is Misleading
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / Code Quality |
| File | .github/workflows/docker.yml : line 13 |
Description
The PR adds the comment # Intentionally unrestricted — allows manual builds from any branch for flexibility to workflow_dispatch. This implies a deliberate architectural choice, but workflow_dispatch without a branches filter is simply the GitHub Actions default — no special configuration was made. The comment may mislead a future maintainer into thinking that removing it would also add a restriction, or that some explicit opt-out of branch filtering was applied.
Code
workflow_dispatch: # Intentionally unrestricted — allows manual builds from any branch for flexibilityRecommendation
Either remove the comment (the default behaviour is already well-documented by GitHub) or replace it with a more precise statement:
workflow_dispatch: # No branch filter — GitHub default; allows manual triggers from any branch[MN-04] BANKOFAISkill.md Filename No Longer Matches Page Title
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality / Documentation |
| File | docs/McpServer-Skills/SKILLS/BANKOFAISkill.md : Line 1 |
Description
The file was renamed from the heading "BANK OF AI Skills" to "Skill Catalog", but the file itself was not renamed. The URL slug generated by Docusaurus will remain /McpServer-Skills/SKILLS/BANKOFAISkill, which conflicts with the displayed page title "Skill Catalog". This creates a confusing breadcrumb and URL for users, and makes it harder to discover via search.
Code
-# BANK OF AI Skills
+# Skill CatalogRecommendation
Rename the file to SkillCatalog.md (or skill-catalog.md) and update all references in sidebars.js, i18n/en/…/sidebars.js, and any cross-links in other docs. Alternatively, add slug: BANKOFAISkill as explicit Docusaurus front matter to preserve the URL while using the new title.
[MN-05] agent-wallet reset Anchor May Not Resolve in CLI Reference
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/FAQ.md, docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md, docs/Agent-Wallet/FAQ.md |
Description
Three separate FAQ files link to ../../../Agent-Wallet/Developer/CLI-Reference#agent-wallet-reset-reset-all-data. However, the CLI Reference diff reviewed does not show an agent-wallet reset section in the basic commands area — the visible diff covers start, sign, add, list, use, inspect, remove, and the non-interactive execution methods, but a dedicated reset heading with the exact slug agent-wallet-reset-reset-all-data is not confirmed. If the anchor is missing, all three deep links will silently land at the top of the CLI Reference page rather than the relevant section, creating a confusing experience for users in exactly the crisis situation (lost password, wallet corruption) where they need the most direct help.
Code
Run `agent-wallet reset` to wipe and start over — see
[CLI Reference → Reset](../../../Agent-Wallet/Developer/CLI-Reference#agent-wallet-reset-reset-all-data)Recommendation
Verify that docs/Agent-Wallet/Developer/CLI-Reference.md contains a heading that generates the anchor agent-wallet-reset-reset-all-data (e.g., ## \agent-wallet reset` (Reset All Data)`). If not, add the section or correct the anchor slug in all three FAQ files.
[MN-06] SKILLS QuickStart Install Script Uses Unverified curl | bash
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Documentation |
| File | docs/McpServer-Skills/SKILLS/QuickStart.md : Step 1 |
Description
The primary installation method instructs users to run curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bash. This fetches and executes arbitrary shell code from a dynamic branch reference (refs/heads/main), without any integrity check (e.g., a checksum or a pinned commit SHA). A supply-chain compromise of the main branch of openclaw-extension would silently affect all users following these instructions. An alternative git-clone path is documented but is presented as secondary.
Code
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bashRecommendation
Either pin the URL to a versioned tag or commit SHA (preferred), or promote the git-clone-inspect path as the primary method:
# Recommended (inspect before running)
git clone https://github.com/BofAI/openclaw-extension.git
cat openclaw-extension/install.sh # inspect first
bash openclaw-extension/install.shIf a dynamic URL is intentional, document that users should run curl ... | cat first and review the script before executing.
[MN-07] SKILLS Intro.md Removes Technical Skill-Structure Documentation
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | docs/McpServer-Skills/SKILLS/Intro.md |
Description
The original Intro.md contained a well-structured explanation of what a Skill file looks like (directory layout, SKILL.md frontmatter schema, scripts/, resources/, package.json). This was the canonical reference for developers wanting to create their own Skills. The rewrite replaces this entirely with a product-marketing introduction aimed at end users. The technical reference is not replicated in BANKOFAISkill.md or any other new file in this PR.
Code (removed)
## What Does a Skill Look Like?
A Skill is a folder with a very simple structure:
| File/Directory | Purpose | Required? |
| `SKILL.md` | Core instruction document | **Yes** |
| `scripts/` | Executable scripts | Optional |
| `resources/` | Reference data | Optional |
| `package.json` | npm dependency declaration | Optional |
The top of `SKILL.md` contains YAML Frontmatter...Recommendation
Add a new Developer/Creating-Skills.md page (or add a "For Developers" section to BANKOFAISkill.md) that preserves the Skill authoring reference — file structure, SKILL.md frontmatter schema, and the guidance on name/description for AI matching. Wire it up in sidebars.js under a collapsed developer sub-section.
Suggestions
[S-01] Add Version Front Matter to New Agent Wallet Docs
File: All 6 new docs/Agent-Wallet/ files
Description: None of the new files include Docusaurus front matter (e.g., sidebar_position, description, keywords). Adding minimal front matter would improve SEO and sidebar ordering reliability.
Suggestion:
---
sidebar_position: 1
description: "Learn how Agent-wallet protects your private keys with a dual-lock encryption model."
keywords: [agent-wallet, private key, encryption, TRON, AI agent]
---[S-02] TypeScript SDK TextEncoder Usage Should Be Verified Against Actual API
File: docs/Agent-Wallet/Intro.md : Developer path section
Description: The TypeScript SDK example in Intro.md passes new TextEncoder().encode("Hello!") (a Uint8Array) to wallet.signMessage(). Most blockchain signing SDKs accept a string or Uint8Array for raw-bytes signing, but if the SDK's signMessage is designed to follow EIP-191 personal sign semantics (which prepends a standard prefix), passing a Uint8Array could produce a different signature than a user expects.
Suggestion: Confirm the SDK interface accepts Uint8Array directly and that this produces the same result as wallet.signMessage("Hello!"). Update the example to match the primary supported usage pattern, and add a comment explaining when to use TextEncoder vs. a plain string.
[S-03] McpServer-Skills/Intro.md Should Reference the New SKILLS QuickStart
File: i18n/en/docusaurus-plugin-content-docs/current/McpServer-Skills/Intro.md
Description: The diff shows McpServer-Skills/Intro.md was modified (8 lines changed). The SKILLS section now has a QuickStart page but it's not clear whether McpServer-Skills/Intro.md links to it. If the top-level MCP intro page doesn't reference the new SKILLS QuickStart, new users may miss the fastest on-ramp.
Suggestion: Ensure McpServer-Skills/Intro.md (both the main and i18n version) contains a direct link to McpServer-Skills/SKILLS/QuickStart.
[S-04] Document Agent Wallet Version Compatibility Requirements
File: docs/Agent-Wallet/Intro.md, docs/Agent-Wallet/QuickStart.md
Description: The docs mention npm install -g @bankofai/agent-wallet but do not specify a minimum version of the package required to support agent-wallet reset, agent-wallet start (vs. the older agent-wallet init), and the --save-runtime-secrets flag. Users with older installations may hit unexpected errors when following new docs.
Suggestion: Add a callout noting the minimum @bankofai/agent-wallet version required for the features documented in this PR, and add agent-wallet --version to the verification step.
[S-05] SKILLS/Faq.md Credentials Configuration Now Only References Agent Wallet — Should Also Cover Fallback
File: docs/McpServer-Skills/SKILLS/Faq.md
Description: The old FAQ had a clear export TRON_PRIVATE_KEY="your_private_key" section. The new FAQ focuses on Agent Wallet as the recommended path, with the plaintext key setup delegated to a link in BANKOFAISkill.md. Users who cannot or choose not to use Agent Wallet (e.g., in CI environments without a wallet file) may not find the plaintext-key fallback easily. The FAQ currently has no answer for "How do I set a raw TRON_PRIVATE_KEY?"
Suggestion: Add a brief "I want to use a raw private key instead" entry that links directly to the relevant section of the Local Deployment docs or BANKOFAISkill quick-start, so users on both paths can self-serve from the FAQ.
Positive Observations
| Area | Observation |
|---|---|
| Security: password echo fix | Replacing echo $AGENT_WALLET_PASSWORD with [[ -n "$AGENT_WALLET_PASSWORD" ]] && echo "set" || echo "not set" across all FAQ pages is the correct fix for the prior audit finding. Consistently applied in both SUN and TRON MCP FAQs. |
| Security: CI debug step removal | Removing the Debug Docker Hub secrets step that printed echo "username=[$DOCKERHUB_USERNAME]" to CI logs eliminates a credential exposure risk. |
| Security: single-quote hygiene | Switching all sensitive export commands from double quotes to single quotes ('your_password') is correct and consistently applied. It prevents shell expansion of $, !, and backtick characters in passwords. |
| Security: AGENT_WALLET_DIR tilde fix | Changing "~/.agent-wallet" to "$HOME/.agent-wallet" correctly fixes tilde expansion in double-quoted strings. Tilde expansion does not occur inside double quotes in POSIX shells. |
| UX: three-tier audience design | Agent-Wallet/Intro.md segments users into casual / CLI / developer paths with distinct entry points. This is excellent information architecture for a product with a wide skill-range audience. |
| UX: prompt-driven skill catalog | Replacing code-block command examples with copy-pasteable natural language prompts in BANKOFAISkill.md significantly lowers the barrier to entry for non-technical users. |
| Documentation: cross-linking | The new Agent Wallet docs are thoroughly cross-linked from existing MCP Server and SKILLS docs. Deep links to specific anchors (e.g., CLI-Reference#non-interactive-execution) are well-targeted. |
| Documentation: admonition usage | :::danger, :::warning, :::caution, and :::tip admonitions are used accurately and consistently to signal severity levels throughout the new Agent Wallet section. |
| Safety: Three Golden Rules preserved | The new BANKOFAISkill.md preserves all three original safety rules (no private keys in chat, testnet first, explicit confirmation for write ops) in a more readable numbered format. No safety information was dropped. |
Documentation: runtime_secrets.json permissions |
The CLI Reference notes that runtime_secrets.json is automatically created with 600 permissions and advises users to verify permissions after manual moves/copies. This is a useful operational detail. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 6 | 5 | 1 | 0 | MN-05: agent-wallet reset anchor unverified |
| Security | 8 | 6 | 2 | 0 | MN-01 (--save-runtime-secrets framing), MN-02 (Method C missing warning) |
| Performance | 3 | 3 | 0 | 3 | N/A for documentation |
| Code Quality | 5 | 3 | 2 | 0 | MN-03 (misleading comment), MN-04 (filename mismatch) |
| Testing | 2 | 2 | 0 | 5 | N/A for documentation |
| Documentation | 6 | 4 | 2 | 0 | MN-06 (unverified curl|bash), MN-07 (lost technical reference) |
| Compatibility | 3 | 2 | 1 | 0 | S-04: no version requirements stated |
| Observability | 2 | 2 | 0 | 5 | N/A for documentation |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analysed only the diff between main and update-mcp-server. Runtime behaviour, integration testing, and deployment impact are not covered. Specifically: link anchors (MN-05) and SDK API signatures (S-02) should be manually verified by a developer with access to the Agent Wallet source.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-23
Code Review ReportProject: BofAI Docs (x402-tron/docs) PR OverviewBranch Information
Commit History (Recent)
Review SummaryVerdict
Findings at a Glance
SummaryThis PR is a significant documentation expansion adding a complete Agent-Wallet section (6 new pages in both English and Chinese), rewriting the SKILLS documentation, and adding a new SKILLS QuickStart page. It also makes a crucial CI security fix (removing a step that leaked Docker Hub credentials to CI logs) and improves environment variable handling guidance across MCP Server docs (switching from double quotes to single quotes for passwords, replacing The overall quality is high — content is comprehensive, well-structured, and security-conscious in its intent. However, two major issues undermine the stated security goals: the new Change Summary1. CI Workflow Security Fix
Purpose: Eliminates a CI step that was echoing 2. New Agent-Wallet Documentation (English + Chinese)
Purpose: Introduces Agent-Wallet as the recommended credential management approach, replacing plaintext private key usage patterns referenced in older docs. 3. Updated MCP Server Documentation
Purpose: Standardizes credential handling guidance, links to the new Agent-Wallet documentation, and fixes previously dangerous instructional patterns. 4. Rewritten Skills Documentation
Purpose: Shifts documentation from developer-focused technical reference to beginner-friendly onboarding content. 5. Navigation and Package Updates
Detailed FindingsMajor[MJ-01] Inconsistent Quoting Advice for
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security / Documentation |
| File | docs/McpServer-Skills/SKILLS/QuickStart.md : Lines 3272–3274, 3281–3283 (diff) |
Description
The new
SKILLS/QuickStart.md"Option 2: Paste Your Private Key Directly" instructs users to use double quotes forTRON_PRIVATE_KEY:export TRON_PRIVATE_KEY="your_real_or_testnet_private_key"It also notes: "
⚠️ Important: Don't forget the double quotes on both sides!" — effectively telling users double quotes are required.This directly contradicts the security fix made elsewhere in the same PR. The MCP Server FAQ files were changed from double quotes to single quotes precisely because special characters (like
$) in a private key string will be silently shell-expanded when wrapped in double quotes, corrupting the value. With double quotes, a key fragment like$abcgets replaced by the value of shell variable$abc, which is typically empty — the resulting key loads silently as a wrong, shorter value and may sign with the wrong key or fail in non-obvious ways.A private key is a random 64-character hex string; while it's unlikely to contain
$, it is not impossible and future guidance may use keys/mnemonics that do contain such characters. More importantly, the Chinese translation of this same file mirrors the same double-quote instruction, amplifying the inconsistency.
Code
+ export TRON_PRIVATE_KEY="your_real_or_testnet_private_key"
+ ⚠️ Important: Don't forget the double quotes on both sides!Compare to the fix made elsewhere in this PR:
-export AGENT_WALLET_PASSWORD="your_password"
+export AGENT_WALLET_PASSWORD='your_password'Recommendation
Change both instances (Mac and WSL) in SKILLS/QuickStart.md to use single quotes, and update the warning text:
export TRON_PRIVATE_KEY='your_real_or_testnet_private_key'
# ✅ Single quotes prevent the shell from expanding any special charactersApply the same fix to i18n/zh-Hans/.../current/McpServer-Skills/SKILLS/QuickStart.md.
[MJ-02] Node.js Version Requirement Inconsistency Across New Documentation
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness / Documentation |
| File | docs/McpServer-Skills/SKILLS/Faq.md : Line 2682 (diff) vs docs/Agent-Wallet/Developer/SDK-Guide.md : Line 924 (diff) |
Description
The new documentation contains contradictory minimum Node.js version requirements:
SKILLS/Faq.md(new content, "Installation failed" section): "Skills require v20 or higher."Agent-Wallet/Developer/SDK-Guide.md(new file): "Requires Node.js ≥ 18."Agent-Wallet/QuickStart.md(new file): "Node.js (a runtime environment, version >= 18)"Agent-Wallet/FAQ.md(new file): "As long as you can run Node.js >= 18 ... you're good to go."Users following the Skills FAQ will install Node.js 20+. Users following the Agent-Wallet docs will install Node.js 18. When a Node.js 18 user then tries to run skills (which require v20), the failure will be cryptic because the QuickStart explicitly told them v18 was sufficient. This creates a confusing, inconsistent experience that undermines trust in the documentation.
Code
<!-- SKILLS/Faq.md -->
Run `node --version` in your terminal. Skills require **v20 or higher**.
<!-- Agent-Wallet/Developer/SDK-Guide.md -->
Requires Node.js ≥ 18.Recommendation
Audit the actual runtime requirements of the skills scripts and Agent-Wallet CLI, then set a single consistent minimum version across all documentation. If skills truly require v20, update Agent-Wallet docs to also require v20 (or document clearly that they have different requirements):
<!-- Consistent across all docs -->
Requires Node.js ≥ 20 (LTS). Check with: `node -v`Minor
[MN-01] AGENT_WALLET_DIR Tilde Expansion Inconsistency — Partially Fixed
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md : Lines 2196–2199 (diff) |
Description
The PR correctly fixes
AGENT_WALLET_DIR="~/.agent-wallet"→AGENT_WALLET_DIR="$HOME/.agent-wallet"in both SUN and TRON deployment docs. Inside double-quoted strings,~is not expanded by the shell, so the old instruction set the variable to the literal string~/.agent-walletrather than the user's home directory path. The fix using$HOMEis correct.However, the new Agent-Wallet SDK Guide (
SDK-Guide.md) still shows:export AGENT_WALLET_DIR="$HOME/.agent-wallet"This is actually correct (double-quoted
$HOMEis expanded). But for full consistency with the single-quote recommendations used for passwords throughout the new documentation, this could cause reader confusion — why are passwords single-quoted but directory paths double-quoted?
Recommendation
This is a minor consistency concern. No functional bug. Document a style guide comment inline or use the same quoting style throughout:
# $HOME needs to expand — double quotes are appropriate here export AGENT_WALLET_DIR="$HOME/.agent-wallet"
[MN-02] curl | bash Installation Pattern Without Pinned Commit
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security |
| File | docs/McpServer-Skills/SKILLS/QuickStart.md : Line 3183 (diff); docs/McpServer-Skills/SKILLS/BANKOFAISkill.md : removed section |
Description
The new
SKILLS/QuickStart.mdpromotes the installation command:curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bashThis fetches and executes the tip of the
mainbranch directly. If the repository is ever compromised, users running this command receive and execute malicious code without any integrity check. Therefs/heads/mainpointer can change at any time.This pattern existed in the old
BANKOFAISkill.mdand was not introduced by this PR. However, the PR rewritesSKILLS/QuickStart.mdto make this the primary and most prominent installation method, elevating its exposure. Notably, the oldBANKOFAISkill.mddid include a "Method 2: check script before running" alternative; that alternative has been removed in the rewrite.
Recommendation
Either pin to a specific commit SHA, or restore the "inspect before running" alternative as a visible option:
# Inspect the script before running (recommended for security-conscious users) curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh -o install.sh # Review install.sh, then: bash install.sh
[MN-03] Stale Reference: agent-wallet init Removed from TRON FAQ but May Still Exist Elsewhere
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md : Line 2232 (diff) |
Description
The PR correctly replaces
agent-wallet initwithagent-wallet reset(plus a new warning about data loss) inTRONMCPServer/FAQ.md. The old commandagent-wallet initno longer exists in the current CLI (agent-wallet startcreates wallets;agent-wallet resetwipes them). However, only the TRON FAQ was updated to referenceagent-wallet reset; the SUN FAQ and the new CLI Reference useagent-wallet resetcorrectly. The fix is consistent across the changed files.The risk is low but the CLI-Reference.md should be verified to not reference
agent-wallet initanywhere. A quick scan of the new content confirmsagent-wallet initdoes not appear in the new documentation, so this is informational only.
Recommendation
No immediate action needed. Add a note to documentation maintenance procedures: when CLI command names change, search the entire docs tree for old names.
[MN-04] Chinese Translation Still Uses Double Quotes for TRON_PRIVATE_KEY
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Documentation |
| File | i18n/zh-Hans/docusaurus-plugin-content-docs/current/McpServer-Skills/SKILLS/QuickStart.md : Lines 6580–6582, 6589–6591 (diff) |
Description
The Chinese translation of
SKILLS/QuickStart.mdmirrors the same double-quote instruction as [MJ-01]. Chinese-language users following the instructionsexport TRON_PRIVATE_KEY="你的真实或测试网私钥"are equally exposed to shell variable expansion issues.
Recommendation
Apply the same single-quote fix to the Chinese translation as recommended in [MJ-01].
Suggestions
[S-01] workflow_dispatch Comment Could Be More Specific
File: .github/workflows/docker.yml : Line 20 (diff)
Description: The comment # Intentionally unrestricted — allows manual builds from any branch for flexibility is clear about intent, but does not explain the security implication (anyone with write access can trigger a build from any branch, potentially triggering image pushes if secrets are available to the job).
Suggestion: Consider adding: # Note: manual trigger only pushes if DOCKERHUB_TOKEN secret is set; PRs do not have access to secrets.
[S-02] SDK-Cookbook broadcastResult Error Handling Could Throw
File: docs/Agent-Wallet/Developer/SDK-Cookbook.md : Lines 470–473 (diff)
Description: In the TRON Transfer TypeScript example, when broadcastResult.result is falsy, the code calls console.error(...) but does not throw or return an error. The function then reaches return signedTx.txID, returning a txID for a failed broadcast. Code that calls transferTRX() and checks the return value would incorrectly conclude the transfer succeeded.
if (broadcastResult.result) {
console.log("Broadcast successful! txID:", signedTx.txID);
} else {
console.error("Broadcast failed:", broadcastResult); // Does not throw!
}
return signedTx.txID; // Returns even on failureSuggestion: Change to throw new Error(...) on broadcast failure. Since this is example/documentation code that users will copy directly, it should model best practices:
} else {
throw new Error("Broadcast failed: " + JSON.stringify(broadcastResult));
}[S-03] Python Cookbook Imports asyncio Inside a Function Body
File: docs/Agent-Wallet/Developer/SDK-Cookbook.md : Line 734 (diff)
Description: The BSC/EVM Python example has import asyncio inside the transfer_bnb function body (not at the top of the file). While this works in Python, it is non-idiomatic and will confuse readers who are new to Python. The comment "NOTE: This blocks the event loop..." is correct and useful, but users may copy only the function body into their own module and get a NameError: name 'asyncio' is not defined error because the import is scoped to the function.
Suggestion: Move import asyncio to the top of the file alongside the other imports to follow standard Python conventions.
Positive Observations
| Area | Observation |
|---|---|
| Security — CI | The PR removes the Debug Docker Hub secrets CI step that was printing credential metadata to workflow logs — a real and impactful security fix. |
| Security — Password Display | Multiple docs replace echo $AGENT_WALLET_PASSWORD (which reveals the password in terminal output) with `[[ -n "$AGENT_WALLET_PASSWORD" ]] && echo "Password is set" |
| Security — Quoting | Consistent upgrade from double quotes to single quotes for password environment variables across SUNMCPServer and TRONMCPServer docs reduces risk of shell expansion corrupting credentials. |
| Security — Password History | New Agent-Wallet docs include detailed warnings about shell history exposure for inline -p flags, and recommend editing .zshrc/.bashrc directly rather than running echo "..." >> to avoid recording secrets in history files. |
Security — raw_secret Warnings |
The new CLI Reference and Agent-Wallet docs consistently use :::danger callouts for raw_secret wallet type, making the security risk prominent and hard to miss. |
| Documentation Depth | SDK-Cookbook provides complete, runnable end-to-end examples (TRON transfer, EVM transfer, x402 payment) in both TypeScript and Python with proper error handling and inline explanatory comments. |
| UX — Beginner Onboarding | QuickStart pages follow a progressive disclosure pattern: zero-risk read-only operations first, then testnet, then mainnet — reducing the chance of accidental mainnet transactions. |
| Correctness — AGENT_WALLET_DIR | The fix from "~/.agent-wallet" to "$HOME/.agent-wallet" corrects a real shell expansion bug in the tilde-in-double-quotes pattern. |
| Documentation — Terminology | Replacing agent-wallet init (obsolete command) with agent-wallet start / agent-wallet reset throughout the changed docs eliminates a source of user confusion. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 6 | 4 | 2 | 0 | Node.js version contradiction; broadcast-no-throw in cookbook example |
| Security | 8 | 6 | 2 | 0 | Double-quote TRON_PRIVATE_KEY; curl-main pipe |
| Performance | 3 | 0 | 0 | 3 | No runtime code changed — docs only |
| Code Quality | 5 | 4 | 1 | 0 | Python in-function import |
| Testing | 2 | 0 | 0 | 2 | No tests applicable — docs only |
| Documentation | 6 | 5 | 1 | 0 | Broken quoting recommendation in QuickStart |
| Compatibility | 2 | 1 | 1 | 0 | Node ≥18 vs ≥20 inconsistency |
| Observability | 1 | 1 | 0 | 0 | Safe password-check pattern added |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between the specified branches (main...update-mcp-server). Runtime behavior, integration testing, link validity, and deployment impact are not covered. All finding line numbers reference diff output positions, not final file line numbers.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-23
Code Review ReportProject: x402-tron/docs PR OverviewBranch Information
Commit History (most recent)
Review SummaryVerdict
Findings at a Glance
SummaryThis PR introduces a substantial new Agent Wallet documentation section (six new files covering Intro, Quick Start, FAQ, CLI Reference, SDK Guide, and SDK Cookbook), expands the SKILLS documentation section with a new QuickStart page, rewrites the Intro and BANKOFAISkill catalog pages for improved accessibility, and updates all MCP Server documentation (TRON and SUN) to cross-reference the new Agent Wallet docs. A version bump ( The security posture of this PR is strongly net-positive. The most significant contribution is the removal of a One Major finding stands out: the new SKILLS QuickStart page promotes an unverified Change Summary1. CI/CD — GitHub Actions Workflow (
|
| File | Change Type | Description |
|---|---|---|
.github/workflows/docker.yml |
Modified | Branch triggers updated; debug credentials step removed; workflow_dispatch comment added |
Purpose: Align the Docker build trigger with the new update-mcp-server branch (replacing ai-bankofai-patch-1). Most critically, removes a debug step that printed Docker Hub credentials to CI log output.
2. Package Version & Sidebar Navigation
| File | Change Type | Description |
|---|---|---|
package.json |
Modified | Version bump 1.2.3 → 1.2.4 |
sidebars.js |
Modified | Added SKILLS/QuickStart entry; added entire Agent Wallet category |
i18n/en/docusaurus-plugin-content-docs/current/sidebars.js |
Modified | Mirror of root sidebars.js |
i18n/en/docusaurus-plugin-content-docs/current.json |
Modified | Added four new Agent-Wallet doc entries |
Purpose: Surface the new documentation sections in the rendered site navigation.
3. New Agent Wallet Documentation Section
| File | Change Type | Description |
|---|---|---|
docs/Agent-Wallet/Intro.md |
New | Conceptual introduction, threat model, dual-lock security model |
docs/Agent-Wallet/QuickStart.md |
New | Step-by-step setup: install, configure env var, test in OpenClaw |
docs/Agent-Wallet/FAQ.md |
New | Answers to common questions including security, offline use, password loss |
docs/Agent-Wallet/Developer/CLI-Reference.md |
New | Full CLI reference including start, sign, add, use, reset |
docs/Agent-Wallet/Developer/SDK-Guide.md |
New | TypeScript/Python SDK integration guide |
docs/Agent-Wallet/Developer/SDK-Cookbook.md |
New | Ready-to-run code recipes for common signing scenarios |
i18n/en/…/Agent-Wallet/* |
New | English i18n mirrors of the above |
Purpose: Provide complete standalone documentation for the @bankofai/agent-wallet package as an alternative to plaintext private keys.
4. Updated SKILLS Documentation
| File | Change Type | Description |
|---|---|---|
docs/McpServer-Skills/SKILLS/Intro.md |
Modified | Rewritten with user-facing narrative; removed technical deep-dive |
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md |
Modified | Retitled "Skill Catalog"; restructured with sample prompts; links to Agent Wallet |
docs/McpServer-Skills/SKILLS/QuickStart.md |
New | Two-step installation guide with curl | bash and fallback options |
docs/McpServer-Skills/SKILLS/Faq.md |
Modified | Rewritten with user-facing Q&A; links to Agent Wallet docs |
Purpose: Shift SKILLS docs from a developer-oriented reference toward an approachable onboarding experience.
5. Updated MCP Server Documentation (TRON & SUN)
| File | Change Type | Description |
|---|---|---|
docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md |
Modified | Updated Agent Wallet password-lost flow; now links to CLI Reference |
docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md |
Modified | Consolidated Agent Wallet setup; links to new Agent Wallet docs |
docs/McpServer-Skills/MCP/TRONMCPServer/ToolList.md |
Modified | list_wallets and select_wallet now link to Agent Wallet Intro |
docs/McpServer-Skills/MCP/SUNMCPServer/FAQ.md |
Modified | Replaced insecure troubleshooting steps with safe alternatives |
docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md |
Modified | Same as TRON counterpart |
i18n/en/…/McpServer-Skills/* |
Modified | English i18n mirrors of the above |
Purpose: Reduce documentation duplication by delegating Agent Wallet setup details to the new dedicated section; incorporate security improvements.
Detailed Findings
Major
[MJ-01] Unverified curl | bash Installation Script
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security |
| File | docs/McpServer-Skills/SKILLS/QuickStart.md : Lines 26–30 |
Description
The new SKILLS Quick Start page instructs all users — explicitly targeting beginners with no prior technical experience — to run a remote shell script directly via curl | bash with no integrity verification (no checksum, no GPG signature). If the GitHub repository, CDN, or any redirect in the network path is compromised, the attacker can serve arbitrary commands that execute with the user's privileges. This is a well-known supply-chain attack vector. The audience (stated as "no private keys, no configuration") makes it higher risk: users have little context to detect anomalies.
Code
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bashRecommendation
Promote the "check script before running" pattern from the old BANKOFAISkill.md (which has now been removed) as the primary recommendation. At minimum, show the git clone + inspect alternative before the curl | bash shortcut, and add a clear security note:
# Recommended: review the script before running
git clone https://github.com/BofAI/openclaw-extension.git
cd openclaw-extension
cat install.sh # inspect before you run
./install.sh
# Quick method (trust the source)
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bashConsider also adding a SHA-256 checksum of a pinned release tag rather than pulling from refs/heads/main (a mutable ref).
Minor
[MN-01] Inline -p Password Flag in CLI Reference Examples
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Lines 29–33 |
Description
The CLI Reference uses agent-wallet start -p Abc12345! as the canonical "custom password" example throughout the document. Although a :::caution Shell history risk admonition is present, the flag value used is a realistic-looking password (Abc12345!), not a clearly synthetic placeholder. Users who copy these examples verbatim for real wallets will expose their actual password in shell history. The existing caution note is good, but could be stronger.
Code
agent-wallet start -p Abc12345!Recommendation
Replace the flag example value with an obviously synthetic placeholder and update the caution note to appear before the code block:
# Caution: passing -p inline records the password in shell history.
# For production wallets, prefer interactive mode (no -p flag).
agent-wallet start -p '<YOUR_STRONG_PASSWORD>'[MN-02] TRON_PRIVATE_KEY Uses Double Quotes in SKILLS QuickStart (Inconsistency)
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Code Quality |
| File | docs/McpServer-Skills/SKILLS/QuickStart.md : Lines 83, 94 |
Description
Across this entire PR, sensitive environment variables (AGENT_WALLET_PASSWORD, TRON_MNEMONIC, TRON_PRIVATE_KEY) have been consistently updated to use single quotes to prevent accidental shell expansion. However, in the new SKILLS QuickStart, the "Option 2: Paste your private key directly" section still uses double quotes for TRON_PRIVATE_KEY. Hex private keys rarely contain shell-special characters, so this is unlikely to cause a runtime breakage, but it is an inconsistency with the rest of the documentation and the stated security guidance.
Code
export TRON_PRIVATE_KEY="your_real_or_testnet_private_key"Recommendation
export TRON_PRIVATE_KEY='your_real_or_testnet_private_key'[MN-03] Misleading workflow_dispatch Comment in CI Workflow
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | .github/workflows/docker.yml : Line 14 |
Description
The added inline comment states # Intentionally unrestricted — allows manual builds from any branch for flexibility. workflow_dispatch does not allow triggering "from any branch" by default — it triggers the workflow from the branch selected in the GitHub Actions UI, which still must have the workflow file present. The phrase "any branch" implies a broader bypass than what workflow_dispatch actually provides, which could confuse future maintainers or create a false sense that there is no branch restriction.
Code
workflow_dispatch: # Intentionally unrestricted — allows manual builds from any branch for flexibilityRecommendation
workflow_dispatch: # Allows manual triggering from the GitHub Actions UI (any branch that has this workflow file)Suggestions
[S-01] Pin curl | bash to a Tagged Release Instead of refs/heads/main
File: docs/McpServer-Skills/SKILLS/QuickStart.md
Description: The install URL fetches from refs/heads/main — a mutable branch ref. If a new commit is pushed to main after the docs are published, users could receive a different script version than what was tested. Pinning to a tagged release (e.g., refs/tags/v1.2.3) guarantees reproducibility and reduces the blast radius if the branch is force-pushed or temporarily contains bad code.
Suggestion: Replace refs/heads/main with a pinned tag, and update the tag when new releases ship: https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/tags/v1.2.3/install.sh.
[S-02] Expose Windows Support Gap More Prominently
File: docs/Agent-Wallet/FAQ.md : Line ~153 (Installation & Environment section)
Description: Windows support is currently addressed only deep in the FAQ ("Windows is not currently supported"). The Quick Start page does not mention this; users who follow it on Windows will hit failures midway through. The SKILLS QuickStart references Windows (WSL) under the private-key option but not for Agent Wallet setup.
Suggestion: Add a :::caution or OS tab at the top of Agent-Wallet/QuickStart.md noting that native Windows is unsupported and directing Windows users to WSL.
[S-03] Add Explicit "No i18n Drift" Note or Automation for i18n Mirrors
File: i18n/en/docusaurus-plugin-content-docs/current/*
Description: The i18n directory contains exact mirrors of docs/**. This PR correctly keeps them in sync, but the duplication creates a persistent risk: future changes to primary docs that are not mirrored to i18n/ will cause content drift, broken anchors, or missing pages in the English i18n locale.
Suggestion: Add a note in CONTRIBUTING.md (or a CI lint check) requiring that edits to docs/ are mirrored to i18n/en/. Alternatively, consider using Docusaurus's docusaurus write-translations mechanism to reduce manual mirroring.
Positive Observations
| Area | Observation |
|---|---|
| Critical Security Fix | Removed the Debug Docker Hub secrets CI step that was printing DOCKERHUB_USERNAME and the length of DOCKERHUB_TOKEN to public GitHub Actions logs. This was the most serious vulnerability in the repository and its removal is the single most impactful change in this PR. |
| Safe Password Verification | echo $AGENT_WALLET_PASSWORD — which reveals the password value in terminal output — has been replaced with `[[ -n "$AGENT_WALLET_PASSWORD" ]] && echo "Password is set" |
| Single-Quote Consistency | All sensitive export statements (AGENT_WALLET_PASSWORD, TRON_PRIVATE_KEY, TRON_MNEMONIC, TRON_MNEMONIC) have been migrated from double quotes to single quotes, preventing silent shell expansion of passwords containing $, !, and similar characters. The caution boxes explicitly explain why. |
$HOME Over ~ in Env Vars |
AGENT_WALLET_DIR="~/.agent-wallet" has been corrected to AGENT_WALLET_DIR="$HOME/.agent-wallet". Tilde (~) is not expanded inside double-quoted strings by POSIX-compatible shells, so the old form would have set the variable to the literal string ~/.agent-wallet rather than the resolved home path. |
| Safe Wallet Reset | rm -rf ~/.agent-wallet/ has been replaced with agent-wallet reset across all MCP Server FAQs. The new command is accompanied by strong warnings about irreversibility, links to the CLI Reference, and guidance on backing up mnemonics before proceeding. |
| Shell History Awareness | The Agent Wallet QuickStart explicitly warns users not to use echo "export ..." >> ~/.zshrc because the password ends up in .zsh_history. It recommends editing the config file in a text editor instead. This is a non-obvious security best practice that is rarely documented. |
raw_secret Danger Warnings |
The raw_secret wallet type (which stores keys unencrypted) is consistently labeled with :::danger admonitions throughout the new CLI Reference and FAQ, ensuring users are not inadvertently guided toward it. |
| Cross-Linking Architecture | Rather than duplicating Agent Wallet setup instructions in every MCP Server doc, the PR restructures them to point to the new canonical Agent Wallet section. This eliminates future drift risk and keeps security guidance centralised and consistent. |
| Security-Concious New Docs | The new Agent-Wallet Intro.md includes a concrete threat model (dependency chain poisoning, "helpful" agent leaks, git history exposure) that gives users genuine, actionable context for why the tool exists — significantly better than a simple feature list. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 4 | 4 | 0 | 4 | Logic, links, and env var quoting are correct |
| Security | 10 | 8 | 1 | 1 | curl | bash without integrity check (MJ-01) |
| Performance | 7 | 0 | 0 | 7 | Documentation-only PR; not applicable |
| Code Quality | 6 | 5 | 1 | 0 | Double-quote inconsistency (MN-02); misleading comment (MN-03) |
| Testing | 7 | 0 | 0 | 7 | Documentation-only PR; not applicable |
| Documentation | 6 | 5 | 1 | 0 | Windows gap not surfaced in Quick Start (S-02) |
| Compatibility | 5 | 4 | 0 | 1 | i18n mirrors kept in sync this PR (S-03 for future risk) |
| Observability | 4 | 0 | 0 | 4 | Documentation-only PR; not applicable |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between origin/main and origin/update-mcp-server. Runtime behavior, integration testing, deployment impact, and the correctness of any linked external URLs are not covered.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-23
Code Review ReportProject: BofAI Docs (bankofai/docs) PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
Summary
Change Summary1. CI/CD Workflow (
|
| Category | Items Checked | Pass | Fail | N/A |
|---|---|---|---|---|
| Correctness | 6 | 5 | 1 | 0 |
| Security | 8 | 7 | 1 | 0 |
| Performance | 3 | 0 | 0 | 3 |
| Code Quality | 6 | 5 | 1 | 0 |
| Testing | 2 | 0 | 0 | 2 |
| Documentation | 8 | 6 | 2 | 0 |
Performance and Testing categories are largely N/A for a documentation-only PR. The correctness fail refers to the double-quote inconsistency (MJ-01). The security fail refers to the same issue (double quotes for variables that may contain special characters). The code quality fail refers to the unrestricted workflow_dispatch scope (MN-01). The two documentation fails refer to the missing Linux tip in QuickStart (MN-02) and the cross-link pointing to Intro rather than QuickStart (MN-03).
Disclaimer
This is an automated code review generated by AI Code Reviewer (Code Review Skill v1.0.0). All findings should be verified by a human reviewer before acting on them. The review is limited to the diff between main and update-mcp-server at the time of analysis (2026-03-23) and does not reflect any changes made after that point. Security findings in documentation are assessed based on the guidance provided to end users; they do not reflect vulnerabilities in production code.
Code Review ReportProject: Bank of AI Documentation (x402-tron/docs) PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR delivers a substantial documentation expansion for the Agent Wallet feature and improves existing MCP Server and SKILLS documentation. The primary additions are six new Markdown files for Agent Wallet ( The overall content quality is high — the documentation is thorough, security warnings are explicit and well-placed, and the dual TypeScript/Python code examples in the SDK guide and cookbook are accurate and idiomatic. The CI workflow change correctly deregisters the old Minor issues found are primarily documentation quality concerns: one environment variable reference uses Change Summary1. CI Workflow (
|
| Category | Status | Notes |
|---|---|---|
| A1. Logic errors / off-by-one | PASS | No logic errors in changed code |
| A2. Null/undefined handling | PASS | SDK examples include post-call validation |
| A3. Error handling | PASS | SDK examples catch and re-throw with context |
| A4. Concurrency / race conditions | N/A | Documentation only; no runtime concurrency changes |
| A5. Resource leaks | PASS | Python aiohttp sessions properly scoped via async with |
| A6. API contract / breaking changes | PASS | Documentation changes only; no API surface modified |
| B1. Injection vulnerabilities | PASS | No SQL/command injection vectors introduced |
| B2. Auth/Authz weakened | PASS | No auth logic changed; security advice improved |
| B3. Input validation | PASS | N/A for documentation |
| B4. Sensitive data / hardcoded secrets | PASS | Debug credential printing removed; no new secrets hardcoded |
| B5. Data exposure | PASS | echo $PASSWORD patterns replaced with non-exposing checks |
| B6. New dependencies | PASS | No new runtime dependencies added |
| C1. N+1 queries | N/A | No database access |
| C2. Missing indexes | N/A | No database access |
| C3. Unbounded operations | N/A | No data fetching in changed code |
| C4. Unnecessary computation | N/A | Documentation only |
| C5. Memory management | N/A | Documentation only |
| D1. Naming clarity | PASS | File names, headings, and variable names are clear |
| D2. Function complexity | PASS | No overly complex functions introduced |
| D3. Duplication | MINOR | English and Chinese docs mirror each other (acceptable for i18n) |
| D4. Dead code | PASS | No dead code; removed debug CI step is a positive |
| D5. Style consistency | MINOR | Double-quoted export in SKILLS/QuickStart.md inconsistent with rest of PR |
| E1. Tests for new logic | N/A | Documentation project; no unit tests applicable |
| E2. Test quality | N/A | Documentation project |
| E3. Edge case coverage | MINOR | Python TRON example missing fromAddress validation vs. TypeScript |
| F1. Public API documentation | PASS | All new docs are public-facing documentation |
| F2. Changed behavior documented | PASS | All behavior changes (wallet reset, password check) reflected in docs |
| F3. Complex logic commented | PASS | Non-obvious patterns (e.g., visible: true address format) are explained inline |
Disclaimer
This is an automated code review generated by an AI code reviewer. While it aims to be thorough and accurate, it may miss context-specific concerns or incorrectly flag non-issues. All findings should be reviewed by a human maintainer before acting on them. The reviewer has only examined changes that appear in the diff between main and update-mcp-server.
Code Review ReportProject: BofAI Docs (x402-tron documentation site) PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR introduces a substantial documentation overhaul for a Docusaurus-based docs site. The primary changes are: (1) a completely new "Agent Wallet" section covering introduction, quick start, CLI reference, SDK guide, and SDK cookbook documentation; (2) significant rewrites of the MCP Server (TRON and SUN) and SKILLS documentation to improve readability and audience breadth; (3) removal of client-specific tab-based configuration examples in favour of a client-agnostic approach; (4) a critical CI security fix removing a step that logged secret values to the workflow console; and (5) a version bump of The content quality is generally high — the new Agent Wallet documentation is well-structured, security-conscious, and user-friendly. The security fix in the CI workflow ( Change SummaryCI / Build
Navigation / Sidebar
New Documentation — Agent Wallet (English + zh-Hans mirrors)
Updated Documentation — MCP Servers
Updated Documentation — SKILLS
Updated Documentation — Openclaw Extension
Detailed FindingsMAJOR-1 — Undocumented
|
| Check | Result | Notes |
|---|---|---|
| Security: No hardcoded secrets | PASS | Debug step with credential logging removed |
| Security: Env var quoting guidance is safe | PARTIAL | Single-quote guidance is strong overall, but SKILLS QuickStart Option 2 uses double-quotes for TRON_PRIVATE_KEY |
| Security: Sensitive var values not echoed to console | PASS | Changed to existence-check pattern |
| CI: Workflow syntax valid | PASS | docker.yml is well-formed YAML |
| Correctness: All referenced files exist | PARTIAL | npx add-mcp and npx skills add reference packages not linked or documented |
| Correctness: Internal cross-links resolve | PARTIAL | CLI Reference anchor #agent-wallet-reset-reset-all-data resolves; OfficialServerAccess still in sidebar and still has content |
| Correctness: i18n sidebar in sync | FAIL | zh-Hans sidebar missing SKILLS/QuickStart entry |
| Documentation: New Agent Wallet section complete | PASS | Intro, QuickStart, CLI Reference, SDK Guide, SDK Cookbook, FAQ all present |
| Documentation: i18n translations present for new content | PASS | All new Agent Wallet and SKILLS files have zh-Hans counterparts |
| Documentation: Frontmatter present | PARTIAL | New Agent Wallet docs lack frontmatter; SUN FAQ retains it |
| Code Quality: No dead code | PASS | Removed import Tabs / import TabItem imports where tabs were removed |
| Testing: Documentation examples are runnable | PARTIAL | npx commands lack version pins; add-mcp / skills packages undocumented |
| Performance: Build impact | PASS | Only documentation content added, no code that affects build performance |
Disclaimer
This report was generated by an automated AI code reviewer. All findings should be verified by a human reviewer before taking action. The AI reviewer focuses on the code changes between the specified branches only and does not have access to runtime environments, external services, or the full git history beyond the diff provided.
Code Review ReportProject: x402-tron/docs PR OverviewBranch Information
Commit History (recent)
Review SummaryVerdict
Findings at a Glance
SummaryThis PR is a substantial documentation overhaul with three main themes: (1) adding a brand-new Agent Wallet documentation section (6 new files + Chinese translations), (2) refactoring the MCP Server docs to be client-agnostic (replacing Claude Desktop / Claude Code / Cursor-specific tabs with generic instructions), and (3) rewriting the SKILLS section intro in a more consumer-friendly tone. A workflow fix also removes a security-sensitive debug step that was leaking Docker Hub credential metadata. The quality of the new Agent Wallet documentation is high — security guidance is thorough and accurate. The client-agnostic direction for MCP Server docs is the right strategic move for a growing ecosystem. However, the refactoring of the local deployment guides has introduced two Major gaps: client configuration instructions were removed from both Change Summary1. CI/CD: GitHub Actions Workflow
Purpose: Remove a debug step that exposed Docker Hub credential metadata ( 2. Navigation: Sidebar Updates
Purpose: Wire up new documentation pages into the site navigation. 3. New Feature: Agent Wallet Documentation Section
Purpose: Document the Agent Wallet tool comprehensively for end users and developers integrating the SDK. 4. MCP Server Docs: Client-Agnostic Refactor
Purpose: Make MCP Server documentation client-agnostic so it applies to any MCP-compatible tool, not just Claude Desktop / Claude Code / Cursor. 5. SKILLS Documentation Rewrite
Purpose: Reposition SKILLS docs from a developer-oriented technical reference to a more accessible consumer-facing guide. 6. Openclaw Extension Docs
Purpose: Refresh and streamline Openclaw extension documentation. Detailed FindingsMajor[MJ-01] Local Deployment Guide Missing Client Configuration (TRON MCP Server)
Description The PR removes "Step 4: Client Configuration" entirely from the TRON MCP Server local deployment guide. The removed step contained concrete configuration examples for Claude Desktop ( Code (removed from diff): Recommendation Restore a client configuration step that is consistent with the new client-agnostic direction. Since the goal is to avoid tool-specific tabs, a minimal generic version would suffice: ### Step 4: Connect Your MCP Client
Run the server and register it with your MCP client. The exact method depends on your client — consult your client's documentation for adding a `stdio` server. Common approaches:
**Using `npx` (any client that supports stdio MCP servers):**
```json
{
"mcpServers": {
"mcp-server-tron": {
"command": "npx",
"args": ["-y", "@bankofai/mcp-server-tron"]
}
}
}After adding the configuration, restart your MCP client. Recommendation After the "Server Running Methods" section, add a "Step 4: Connect Your MCP Client" section with at minimum a generic JSON snippet showing how to register the running server as an MCP stdio server, and a pointer to each client's docs for specifics. Minor[MN-01] QuickStart Uses Claude Code-Specific
|
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/QuickStart.md : line ~25 |
Description
The SUN MCP QuickStart was changed from a tabbed guide (one tab each for Claude Desktop, Claude Code, Cursor) to a single command:
-<Tabs>
-<TabItem value="claude-desktop" label="Claude Desktop">
- ...
-</TabItem>
-<TabItem value="claude-code" label="Claude Code">
- Run the following command in the terminal to add SUN MCP Server:
- ```bash
- claude mcp add --transport http sun-mcp-server https://sun-mcp-server.bankofai.io/mcp
- ```
-</TabItem>
-...
+Run the following command in the terminal to add SUN MCP Server:
+
+```bash
+npx add-mcp @bankofai/sun-mcp-server
+```
+After the command completes, restart your MCP client.npx add-mcp is a package provided by Anthropic that is designed specifically for Claude Code's MCP management. Users of Claude Desktop, Cursor, or other generic MCP clients will not benefit from this command and may be confused when it doesn't register the server in their client. This contradicts the PR's stated goal of being client-agnostic.
Recommendation
Replace or supplement the npx add-mcp command with the more universal JSON snippet approach, or add a note that this command targets Claude Code specifically and link to generic HTTP configuration for other clients:
**For Claude Code users:**
```bash
npx add-mcp @bankofai/sun-mcp-serverFor other MCP clients, add this to your client's MCP server configuration:
{
"mcpServers": {
"sun-mcp-server": {
"command": "npx",
"args": ["mcp-remote", "https://sun-mcp-server.bankofai.io/mcp"]
}
}
}
---
#### [MN-02] TRON QuickStart Also Uses `npx add-mcp` Without Alternate Instructions
| Property | Value |
|----------|-------|
| **Severity** | Minor |
| **Category** | Documentation |
| **File** | `docs/McpServer-Skills/MCP/TRONMCPServer/QuickStart.md` |
**Description**
Same issue as MN-01 — the TRON MCP Server QuickStart was also simplified to `npx add-mcp @bankofai/mcp-server-tron` as the sole installation method. Users of non-Claude Code clients receive no usable guidance.
**Recommendation**
Apply the same fix as MN-01: provide a generic JSON configuration block alongside or instead of the `npx add-mcp` command.
---
#### [MN-03] `workflow_dispatch` Is Now Truly Unrestricted — Comment Is Correct but Warrants Awareness
| Property | Value |
|----------|-------|
| **Severity** | Minor |
| **Category** | Compatibility / Observability |
| **File** | `.github/workflows/docker.yml` : line 16 |
**Description**
```diff
- workflow_dispatch:
+ workflow_dispatch: # Intentionally unrestricted — allows manual builds from any branch for flexibility
workflow_dispatch without branch restrictions allows any repository collaborator with write access to manually trigger a Docker image build from any branch. The comment correctly documents the intent. However, this could lead to unintended image builds from experimental or insecure branches being pushed to Docker Hub if access control is not tight. This is a low-risk observation given the explicit comment, but worth noting for future security reviews.
Recommendation
Consider restricting image publication to only protected branches by checking github.ref in the push step:
- name: Build and push Docker image
if: github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master')
...This way manual workflow_dispatch builds can still be triggered (for testing) but images are only published from main/master.
[MN-04] Hardcoded Example Password Unchanged in CLI-Reference Code Block
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / Security |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : lines ~30–35, ~170 |
Description
The new CLI Reference uses Abc12345! as the example password throughout multiple code blocks:
agent-wallet start -p Abc12345!
agent-wallet start -p Abc12345! -k your-private-key-hex
agent-wallet sign msg "Hello" -n tron -p 'Abc12345!'While this is a documentation example (not a hardcoded secret), using a specific-looking password like Abc12345! that satisfies common password policies could lead less experienced users to adopt it as a template, choosing something very close to it (e.g., Abc12345@, Abc12345#) for their real wallets. The same page correctly warns against the -p inline approach in production — the example value is fine in that context, but could be made more obviously synthetic.
Recommendation
Replace with a clearly placeholder value such as YourP@ssw0rd! or <your-password> (noting that <> cannot be used in bash without escaping) to reduce the chance of users treating it as a secure template:
agent-wallet start -p 'YourStr0ngP@ss!'Suggestions
[S-01] SKILLS/Intro.md: Technical Architecture Details Removed
File: docs/McpServer-Skills/SKILLS/Intro.md
Description: The previous version of Intro.md contained a detailed technical explanation of the three-tier progressive loading architecture (Shelf Index → Operation Manual → Tool Execution), the difference between Skills and MCP Server, and the explicit/implicit invocation modes. All of this was replaced with consumer-facing marketing content. Developers looking to understand how Skills work technically or looking to build/integrate Skills will have no reference point.
Suggestion: Consider adding a separate "For Developers" or "Architecture" page under SKILLS that preserves the technical content. The consumer-friendly intro is appropriate for new users, but the prior technical content was valuable for contributors and integrators.
[S-02] OfficialServerAccess.md Pages No Longer Cover Client Configuration
File: docs/McpServer-Skills/MCP/SUNMCPServer/OfficialServerAccess.md, docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md
Description: Both OfficialServerAccess.md files now only contain HTTP API documentation (the former "Generic HTTP Call" tab). The per-client configuration (Claude Desktop, Claude Code, Cursor) was removed. This means the only way to learn how to configure a specific client is through the QuickStart pages, which themselves only cover npx add-mcp (Claude Code). The OfficialServerAccess page is a natural destination for users who want to configure a cloud connection in their specific client, but that information no longer exists anywhere in the docs.
Suggestion: Either add a brief generic MCP client configuration section to each OfficialServerAccess.md, or add a prominent link to a dedicated "MCP Client Setup" guide that gives configuration examples for major clients (Claude Desktop, Cursor, etc.).
[S-03] Chinese (zh-Hans) i18n Files Should Be Reviewed for Accuracy
File: i18n/zh-Hans/docusaurus-plugin-content-docs/current/ (27 files changed)
Description: All English documentation changes were propagated to zh-Hans translations. Given the volume (27 files, thousands of lines), it is not possible to verify translation accuracy in this review. The translations may contain automated-translation artifacts, particularly in the new Agent Wallet and SDK Cookbook sections which include highly technical content (encryption algorithms, Keystore V3, scrypt, EIP-712, etc.).
Suggestion: Have a native Chinese speaker review at minimum the new Agent Wallet documentation translations before merge, especially security-sensitive sections (FAQ security answers, CLI Reference danger/caution callouts).
Positive Observations
| Area | Observation |
|---|---|
| Security fix | The debug step in docker.yml that printed echo "username=[$DOCKERHUB_USERNAME]" and token length was removed — this was a data-leak risk in CI logs and its removal is correct. |
| Password quoting | AGENT_WALLET_PASSWORD='<your-master-password>' correctly changed from double to single quotes in LocalPrivatizedDeployment guides — prevents shell expansion of special characters in passwords. |
$HOME vs ~ |
AGENT_WALLET_DIR="$HOME/.agent-wallet" correctly replaces "~/.agent-wallet" — tilde is not reliably expanded in double-quoted strings. |
| Agent Wallet security docs | The new CLI Reference, FAQ, and SDK Guide contain thorough, accurate security warnings: raw_secret dangers, shell history leakage via -p, runtime_secrets.json risks, dual-lock threat model explanation. These are well-written and technically accurate. |
| Agent Wallet SDK Cookbook | The end-to-end examples (TRON transfer, EVM transfer, x402 payment) include address mismatch validation, JSON parse error handling, and missing field checks that go beyond minimal examples — good defensive coding practice demonstrated. |
| Client-agnostic direction | Replacing hard-coded "Claude Desktop" references with "MCP client" throughout FAQs and intros is the right long-term direction for a growing multi-client ecosystem. |
| Links updated | External GitHub link to agent-wallet docs replaced with internal /Agent-Wallet/Intro link — better for keeping users within the documentation site. |
| Version bump | package.json version correctly incremented from 1.2.3 → 1.2.4 alongside the documentation changes. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 8 | 7 | 1 | 0 | QuickStart npx add-mcp only works for Claude Code (MN-01, MN-02) |
| Security | 10 | 9 | 0 | 1 | Docker debug step removed (positive). No secrets/tokens in new docs. |
| Performance | 7 | 0 | 0 | 7 | No runtime code changes |
| Code Quality | 8 | 7 | 0 | 1 | $HOME fix, single-quote fix applied correctly |
| Testing | 7 | 0 | 0 | 7 | Documentation-only changes |
| Documentation | 6 | 4 | 2 | 0 | Missing client config in local deployment guides (MJ-01, MJ-02) |
| Compatibility | 5 | 3 | 1 | 1 | workflow_dispatch unrestricted (MN-03); npx add-mcp client-specific (MN-01) |
| Observability | 4 | 3 | 0 | 1 | workflow_dispatch comment added for intent clarity |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between main and update-mcp-server. Runtime behavior, integration testing, and deployment impact are not covered. Chinese translation accuracy was not verified.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-23
Code Review ReportProject: x402-tron/docs (BofAI Documentation Site) PR OverviewBranch Information
Commit History (Recent)
Review SummaryVerdict
Findings at a Glance
SummaryThis PR is primarily a documentation overhaul for the BofAI Docusaurus site, with one CI workflow change and a version bump. The changes deliver three major goals: (1) adding a new "Agent Wallet" documentation section with six new files covering intro, quick start, FAQ, CLI reference, SDK guide, and cookbook; (2) significantly simplifying MCP server setup instructions for both TRON and SUN servers, moving from per-client tabbed configurations to a streamlined single-command approach; and (3) rewriting the SKILLS documentation with a new QuickStart page. The CI change is a genuine security improvement — it removes a debug step that was leaking Docker Hub credentials to CI logs. The documentation changes also incorporate several important security fixes: shell quoting improvements for passwords and tilde expansion corrections for paths. The two Major concerns both relate to undocumented Change Summary1. CI/CD Workflow Update
Purpose: Keep the Docker build CI aligned with the active development branch; remove a credential-leaking debug step accidentally left in production. 2. Package Version Bump
Purpose: Semver bump associated with this documentation release. 3. New Agent Wallet Documentation Section
Purpose: Introduce Agent Wallet as a standalone documentation section, replacing scattered install instructions in MCP server docs with a centralised reference. 4. MCP Server Documentation Simplification
Purpose: Reduce maintenance burden of per-client configuration examples; align with new Agent Wallet documentation hub; make docs client-agnostic. 5. SKILLS Documentation Rewrite
Purpose: Move SKILLS onboarding to a simpler UX: copy a prompt, hit enter, done. 6. Openclaw Extension Documentation Rewrite
Purpose: Lower barrier to entry by replacing advanced technical troubleshooting with plain-language guidance. Detailed FindingsMajor[MJ-01]
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security / Correctness |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/OfficialServerAccess.md : Lines 26–28; docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md : same section; plus all i18n counterparts |
Description
Both
OfficialServerAccess.mdfiles now instruct users to connect to the cloud MCP server with a single command:npx add-mcp https://sun-mcp-server.bankofai.io/mcp # or npx add-mcp https://tron-mcp-server.bankofai.io/mcp
add-mcpis not a well-known or verified npm package. The old implementation usedmcp-remote(a documented package). The newadd-mcppackage name is ambiguous:
- If no such package exists on npm, users will receive
npm ERR! 404and the documentation becomes broken.- If the package does exist but is maintained by a third party, users are being instructed to execute unvetted third-party code with no package scope (e.g., no
@bankofai/add-mcp), exposing them to a supply chain attack if the name is squatted.- There is no explanation in the documentation of what
add-mcpis, where it comes from, or what it does — violating the principle of minimum surprise for security-sensitive operations involving wallets and on-chain assets.
Code
Simply tell your AI Agent to execute the following command:
```bash
npx add-mcp https://sun-mcp-server.bankofai.io/mcp
**Recommendation**
Use a scoped package name and link to its source so users can verify it before running. If `add-mcp` is an internal/official tool:
```bash
# Use scoped name with verified provenance
npx @bankofai/add-mcp https://sun-mcp-server.bankofai.io/mcp
Alternatively, if the intention is to use the official @modelcontextprotocol/add-mcp CLI, reference that explicitly. Add a note such as: "add-mcp is published at [npm link] and maintained by [org]."
[MJ-02] npx skills add — Unresolvable or Unverified npm Package
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security / Correctness |
| File | docs/McpServer-Skills/SKILLS/QuickStart.md : Lines 17–19; i18n counterpart |
Description
The new
SKILLS/QuickStart.mdinstructs users to install all BANK OF AI skills with:npx skills add https://github.com/BofAI/skills
skillsis not a standard or published npm package. As written,npx skills add ...will fail with "command not found" for any user who does not already have askillsbinary on their PATH. The documentation provides no context for where this command comes from.Beyond the broken UX, if a malicious
skillspackage is published to npm before the official one, any user following these instructions would execute attacker-controlled code. This is particularly concerning because the SKILLS system is described as managing wallets that hold real on-chain assets.
Code
```bash
npx skills add https://github.com/BofAI/skillsA bunch of text will scroll across the screen — you can ignore it.
**Recommendation**
Either (a) use a fully scoped package name that is already published:
```bash
npx @bankofai/skills add https://github.com/BofAI/skills
Or (b) if skills is a CLI that users install globally as part of the Agent Wallet or OpenClaw setup flow, point to that prerequisite step explicitly before listing this command. Add a link to the package's npm page for user verification.
Minor
[MN-01] workflow_dispatch Unrestricted — Any Branch Can Trigger Docker Hub Push
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security |
| File | .github/workflows/docker.yml : Line 13 |
Description
The workflow comment added in this PR explicitly documents the intent: "Intentionally unrestricted — allows manual builds from any branch for flexibility."
When
workflow_dispatchis unrestricted andgithub.event_name != 'pull_request'is the only gate for the Docker Hub push, any repository member withworkflow_dispatchpermission can push an image built from an arbitrary branch (including feature branches containing experimental or broken changes) directly to the productionbankofai/docsDocker Hub image. In a small trusted team this may be acceptable, but it is an architectural risk worth tracking.
Code
workflow_dispatch: # Intentionally unrestricted — allows manual builds from any branch for flexibilityRecommendation
Add an environment-based restriction or branch guard for the push step, so that manual workflow_dispatch runs can still build locally but only push to Docker Hub when triggered on main or master:
- name: Build and push Docker image
if: github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master' || github.event_name == 'push')If unrestricted manual pushes are truly desired, document this risk in the repository's security policy.
[MN-02] LocalPrivatizedDeployment Step Numbers Inconsistent After Restructure
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / Correctness |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md : Step numbering section |
Description
In the TRON
LocalPrivatizedDeployment.md, after removing Step 4 (Client Configuration) and renaming the old Step 5 to Step 4, the section that was "Step 4: Client Configuration" is now missing. However, the restructured document still refers to "Step 4: Verify Connection" without a preceding "Step 4: Client Configuration" — users are left to find client configuration guidance from the brief inline note. Specifically, the step ordering jumps:
- Step 1: Prerequisites
- Step 2: Choose wallet type
- Step 3: Clone and build (old Step 3 retained)
- (missing: explicit client configuration step)
- Step 4: Verify Connection (previously Step 5)
This is a usability issue that may leave users unable to connect.
Recommendation
Either restore a concise Step 4 covering the most common client configuration pattern, or renumber accurately and add a clear cross-reference:
### Step 4: Connect Your MCP Client
For step-by-step client configuration instructions, refer to your MCP client's documentation.
A quick reference for common clients:
- **Claude Code**: `claude mcp add mcp-server-tron -- npx -y @bankofai/mcp-server-tron`
- **Other clients**: Point to the server command `npx -y @bankofai/mcp-server-tron`[MN-03] CLI-Reference.md — Inline -p Password Caution Is Insufficient
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Documentation |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : ~Lines 37–43 |
Description
The CLI Reference correctly warns that
agent-wallet start -p Abc12345!records the password in shell history, and recommends using interactive mode or theAGENT_WALLET_PASSWORDenv var. However, it then immediately shows code examples using-p Abc12345!in multiple places, including import-key and import-mnemonic examples. The example passwordAbc12345!is also a weak sample that users may copy verbatim.Additionally, the
:::cautionblock warns about shell history but doesn't mention that the password also appears in process listings (ps aux) while the command runs — a risk in multi-user environments.
Code
agent-wallet start -p Abc12345!
# ...
agent-wallet start -p Abc12345! -k your-private-key-hex
agent-wallet start -p Abc12345! -m "word1 word2 word3 ..."Recommendation
Replace the inline -p Abc12345! examples with $AGENT_WALLET_PASSWORD or $(read -s -p "Password: " pw && echo "$pw") to demonstrate the secure pattern:
# Secure: read password from environment variable
AGENT_WALLET_PASSWORD="$AGENT_WALLET_PASSWORD" agent-wallet start
# Or: use interactive mode and let the tool prompt
agent-wallet startIf -p examples are kept for illustration, use a placeholder that is obviously not a real password (e.g., <YOUR_STRONG_PASSWORD>) rather than a realistic-looking value.
[MN-04] FAQ Inconsistency — agent-wallet reset vs agent-wallet init Cross-Referenced
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / Correctness |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md : ~Line 96; docs/McpServer-Skills/MCP/SUNMCPServer/FAQ.md : ~Line 143 |
Description
The new FAQ text correctly uses
agent-wallet reset(the new command name) and cross-links to the CLI Reference. However, the old TRON FAQ still contained a reference toagent-wallet initthat was replaced in this PR. The replacement is correct, but the SUN and TRON FAQs link to an anchor#agent-wallet-reset-reset-all-datain the CLI Reference. If the CLI Reference's actual heading text differs from this anchor, users will land on a broken link. The anchor was not verified to exist in the generatedCLI-Reference.mdcontent.
Recommendation
Verify the heading in CLI-Reference.md generates the anchor agent-wallet-reset-reset-all-data correctly. If the heading is ## \agent-wallet reset` (Reset All Data), Docusaurus would generate #agent-wallet-reset-reset-all-data. Confirm this and add an automated link-check step or use the Docusaurus ` anchor tag to guarantee stability.
Suggestions
[S-01] Parallel i18n Maintenance is Error-Prone
File: i18n/en/docusaurus-plugin-content-docs/current/ (all files)
Description: Every documentation file exists in two copies: docs/ and i18n/en/docusaurus-plugin-content-docs/current/. This PR correctly updated both, but the duplication is a maintenance liability — a future PR touching one but not the other will silently create documentation drift. The diff shows several minor inconsistencies between the two copies (e.g., minor line count differences in some files).
Suggestion: Evaluate whether the i18n English copy could be configured as a symlink or re-exported from the canonical docs/ directory, reducing the surface area for drift. Alternatively, add a CI lint step that diffs docs/ against i18n/en/.../current/ and alerts on divergence.
[S-02] raw_secret Wallet Type Visibility Risk
File: docs/Agent-Wallet/Developer/CLI-Reference.md : ~Lines 21–28
Description: The interactive wizard for agent-wallet start surfaces raw_secret as an option, which stores keys unencrypted. The CLI Reference warns users with a :::danger admonition, which is appropriate. However, docs/Agent-Wallet/FAQ.md includes a comparison table showing raw_secret alongside local_secure, which may normalise the option for casual readers.
Suggestion: In FAQ.md, rename the comparison header from "What's the difference between local_secure and raw_secret?" to "Why you should never use raw_secret" to frame it as a discouragement, not a feature comparison. Alternatively, remove raw_secret from the comparison table entirely and only mention it in the developer CLI Reference with the existing danger callout.
[S-03] Missing Security Warning on AGENT_WALLET_DIR="$HOME/.agent-wallet" Default Visibility
File: docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md : ~Lines 72–75; docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md : same
Description: The env-var block correctly fixed ~/.agent-wallet to $HOME/.agent-wallet (tilde does not expand inside double-quoted assignment). However, neither location notes that the wallet directory itself ($HOME/.agent-wallet/) is only protected by filesystem permissions. If the home directory is world-readable, or if backup tools copy it to cloud storage, the encrypted keystore file could be exfiltrated.
Suggestion: Add a brief :::tip note such as: "Ensure your wallet directory has restricted permissions: chmod 700 ~/.agent-wallet. If you use cloud backup tools (Dropbox, Time Machine, etc.), exclude ~/.agent-wallet from sync."
Positive Observations
| Area | Observation |
|---|---|
| Security fix: credentials in CI | docker.yml removes the Debug Docker Hub secrets step that was printing username=[...] and token_length=... to CI logs — a real credential hygiene improvement. |
| Shell quoting hardening | All three LocalPrivatizedDeployment.md files (TRON, SUN, and i18n copies) correctly changed export AGENT_WALLET_PASSWORD="..." to single quotes, preventing unexpected shell expansion of $, !, and backtick characters in passwords — a direct fix for an elevated security risk. |
| Tilde expansion fix | AGENT_WALLET_DIR="~/.agent-wallet" → AGENT_WALLET_DIR="$HOME/.agent-wallet" is technically correct; tilde ~ does not expand inside double-quoted assignment strings in bash, so the old default path would have been set literally as ~/.agent-wallet. |
| Safe password existence check | FAQs replaced echo $AGENT_WALLET_PASSWORD (which prints the password to the terminal) with [[ -n "$AGENT_WALLET_PASSWORD" ]] && echo "Password is set" — a best practice that confirms configuration without exposing the credential. |
| Client-agnostic language | Removing Claude Desktop / Cursor / Cursor-specific config tabs reduces per-client maintenance burden and makes the docs more future-proof as new MCP clients emerge. |
| Centralised Agent Wallet reference | Replacing repeated inline install instructions with a link to ../../../Agent-Wallet/Intro is a clean single-source-of-truth approach that will be easier to keep accurate. |
Danger callout for raw_secret |
The CLI Reference uses :::danger to clearly discourage use of the raw_secret wallet type for real funds — strong and appropriately placed. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 8 | 6 | 2 | 0 | npx add-mcp / npx skills add may fail; step numbering gap in TRON local deployment |
| Security | 10 | 8 | 2 | 0 | Unverified npx packages (MJ-01, MJ-02); otherwise improved over main |
| Performance | 7 | 0 | 0 | 7 | No runtime code changed |
| Code Quality | 10 | 8 | 2 | 0 | i18n duplication risk; CLI examples show insecure password flag pattern |
| Testing | 7 | 0 | 0 | 7 | Documentation-only changes |
| Documentation | 6 | 4 | 2 | 0 | New Agent Wallet section comprehensive; two broken/unverified anchor links |
| Compatibility | 5 | 5 | 0 | 0 | Docusaurus config additions are additive; no breaking changes |
| Observability | 4 | 0 | 0 | 4 | No runtime code changed |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between the specified branches. Runtime behavior, integration testing, and deployment impact are not covered. The npx add-mcp and npx skills add package concerns (MJ-01, MJ-02) should be validated against the actual npm registry and the team's intended package publishing strategy before this PR is merged.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-23
Code Review ReportProject: BofAI/docs (x402-tron documentation site) PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR makes substantial improvements to the BANK OF AI documentation site. The primary focus is the introduction of a brand-new The security guidance throughout the new Agent-Wallet documentation is exemplary — it correctly advises using single-quoted shell assignments to prevent variable interpolation, cautions against One major documentation issue stands out: the SKILLS Change Summary1. CI/CD Workflow (
|
| Category | Status | Notes |
|---|---|---|
| Correctness & Logic | Pass (with minor issues) | Step numbering gap in SUNMCPServer LocalPrivatizedDeployment; potential broken anchor fragment |
| Security | Pass (with major issue) | Double-quoted TRON_PRIVATE_KEY export contradicts single-quote guidance; CI debug secrets step correctly removed |
| Performance | N/A | Documentation-only changes; no runtime code changed |
| Code Quality | Pass (with suggestions) | Commit messages are poor; emoji-heavy content inconsistent with other docs |
| Testing | N/A | No automated tests for documentation content; build/render verification is the applicable test |
| Documentation & Maintainability | Pass (with suggestions) | New Agent-Wallet section is thorough; removal of per-client configs reduces maintainability for manual users |
Disclaimer
This is an automated code review generated by AI Code Reviewer. While this review aims to be thorough and accurate, it may miss context-specific issues or incorrectly flag items due to limited understanding of project-specific conventions. All findings should be reviewed by a human developer before action is taken. This review covers only the diff between main and update-mcp-server — unchanged code was not audited.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-24
Code Review ReportProject: BofAI Docs ( PR OverviewBranch Information
Commit History (Recent)
Review SummaryVerdict
Findings at a Glance
SummaryThis PR introduces a new Agent Wallet documentation section (6 new files covering an intro, quick start, FAQ, CLI reference, SDK guide, and SDK cookbook), substantially rewrites the MCP Server (TRON and SUN) and SKILLS documentation, and simplifies the Openclaw extension guide. The overarching intent is to adopt a unified However, three major issues require attention before merge. First, the Change Summary1. CI/CD Workflow Update (
|
| File | Change Type | Description |
|---|---|---|
.github/workflows/docker.yml |
Modified | Branch trigger renamed, debug step removed, comment added to workflow_dispatch |
Purpose: Aligns the Docker build trigger to the new update-mcp-server branch and removes a debug step that surfaced Docker Hub credential metadata in CI logs.
2. New Agent Wallet Documentation (6 new files)
| File | Change Type | Description |
|---|---|---|
docs/Agent-Wallet/Intro.md |
Added | Concept introduction — threat model, dual-lock mechanism, comparison table |
docs/Agent-Wallet/QuickStart.md |
Added | End-to-end walkthrough: install, create wallet, configure env var, restart |
docs/Agent-Wallet/FAQ.md |
Added | Three top concerns + everyday use, wallet types, security, install FAQ |
docs/Agent-Wallet/Developer/CLI-Reference.md |
Added | Complete CLI command reference |
docs/Agent-Wallet/Developer/SDK-Guide.md |
Added | TypeScript and Python SDK walkthrough with two operating modes |
docs/Agent-Wallet/Developer/SDK-Cookbook.md |
Added | End-to-end runnable scripts for TRON transfer, BSC transfer, x402 payment |
Purpose: Provides authoritative documentation for the Agent Wallet product across beginner (Intro, QuickStart, FAQ) and developer (CLI, SDK Guide, SDK Cookbook) audiences.
3. MCP Server Docs Simplified — TRON & SUN
| File | Change Type | Description |
|---|---|---|
docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md |
Modified | Replaced per-client tab configs with npx add-mcp walkthrough |
docs/McpServer-Skills/MCP/TRONMCPServer/QuickStart.md |
Modified | Same simplification |
docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md |
Modified | Removed Step 4 per-client config examples; replaced with vague "Configuration Notes" |
docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md |
Modified | Made client-agnostic; condensed wallet troubleshooting; links to Agent-Wallet docs |
docs/McpServer-Skills/MCP/TRONMCPServer/Intro.md |
Modified | Minor wording |
docs/McpServer-Skills/MCP/TRONMCPServer/ToolList.md |
Modified | Minor wording |
docs/McpServer-Skills/MCP/SUNMCPServer/OfficialServerAccess.md |
Modified | Same npx add-mcp simplification |
docs/McpServer-Skills/MCP/SUNMCPServer/QuickStart.md |
Modified | Same simplification |
docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md |
Modified | Similar condensation |
docs/McpServer-Skills/MCP/SUNMCPServer/FAQ.md |
Modified | Client-agnostic; better password-check idiom; links to CLI Reference |
docs/McpServer-Skills/MCP/Intro.md |
Modified | Minor wording |
Purpose: Move away from tool-specific instructions (Claude Desktop / Claude Code / Cursor tabs) toward a single npx add-mcp <url> command that works across any compatible MCP client.
4. SKILLS Documentation Rework
| File | Change Type | Description |
|---|---|---|
docs/McpServer-Skills/SKILLS/QuickStart.md |
Added | New file: npx skills add installation walkthrough |
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md |
Modified | Renamed to "Skill Catalog"; replaced installation instructions with prompt examples |
docs/McpServer-Skills/SKILLS/Intro.md |
Modified | Rewording |
docs/McpServer-Skills/SKILLS/Faq.md |
Modified | Rewording |
Purpose: Simplify skill installation to a single npx skills add command and focus the catalog page on prompts users can copy-paste.
5. Openclaw Extension Docs Rewrites
| File | Change Type | Description |
|---|---|---|
docs/Openclaw-extension/QuickStart.md |
Modified | Full rewrite; gamified tone, step-by-step wizard walkthrough |
docs/Openclaw-extension/FAQ.md |
Modified | Condensed; plain-English troubleshooting |
docs/Openclaw-extension/Intro.md |
Modified | Updated |
Purpose: Lower the barrier for first-time users by reframing technical steps as a "mini-game with 4 levels."
6. Navigation & Package
| File | Change Type | Description |
|---|---|---|
sidebars.js |
Modified | Added Agent Wallet category; added SKILLS QuickStart entry |
package.json |
Modified | Version bump 1.2.3 → 1.2.4 |
i18n/ (multiple) |
Modified | Mirror of all English doc changes above |
Detailed Findings
Major
[MJ-01] LocalPrivatizedDeployment.md — Step 4 Client Configuration Removed Without Replacement
| Property | Value |
|---|---|
| Severity | Major |
| Category | Documentation / Correctness |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md : Lines ~155–185 (new) |
Description
The diff removes the entire "Step 4: Client Configuration" section — which contained complete, working JSON/CLI examples for Claude Desktop, Claude Code, and Cursor (Options A–C) — and replaces it with two vague bullet points under "Configuration Notes." The replacement text does not show what the JSON should look like, does not reference the correct config file paths, and does not address the
envblock needed when system env vars are not inherited. The step count also decrements silently (Step 5 "Verify Connection" becomes Step 4), which could confuse users following numbered steps.The SUN MCP Server equivalent (
SUNMCPServer/LocalPrivatizedDeployment.md) has the same problem.
Code (removed — what was there before)
// Claude Desktop — claude_desktop_config.json
{
"mcpServers": {
"mcp-server-tron": {
"command": "npx",
"args": ["-y", "@bankofai/mcp-server-tron"],
"env": {
"AGENT_WALLET_PASSWORD": "YOUR_PASSWORD (Or set in system env)",
"TRONGRID_API_KEY": "YOUR_KEY_HERE (Or set in system env)"
}
}
}
}Code (what replaced it)
#### Configuration Notes
If you're configuring an MCP client to point to your local server:
- **If running via npx or source**: Use the appropriate command in your MCP client's
configuration (e.g., `command: npx` with `args: ["-y", "@bankofai/mcp-server-tron"]`)
- **If running in HTTP mode**: Point your client to `http://localhost:3001/mcp` via
the HTTP URL configuration optionRecommendation
Restore concrete JSON examples (or link to a dedicated "Client Configuration" reference page). At minimum, show the Claude Desktop / Claude Code / Cursor configs that were there before, or clearly direct users to a page that contains them. The env section note (which warned about not committing secrets to version control) should also be retained.
[MJ-02] npx add-mcp and npx skills add Execute Unversioned Remote Packages — Supply Chain Risk Not Disclosed
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md, docs/McpServer-Skills/MCP/SUNMCPServer/OfficialServerAccess.md, docs/McpServer-Skills/MCP/TRONMCPServer/QuickStart.md, docs/McpServer-Skills/MCP/SUNMCPServer/QuickStart.md, docs/McpServer-Skills/SKILLS/QuickStart.md |
Description
All new installation flows instruct users to run:
npx add-mcp https://tron-mcp-server.bankofai.io/mcp npx skills add https://github.com/BofAI/skills
npx(without a pinned version) downloads and executes the latest published package from npm at the time of the command. Ifadd-mcporskillsis compromised — by a typosquatting attack, a hijacked npm account, or a malicious dependency update — users will silently execute attacker-controlled code in their local shell. This risk is multiplied by the fact that these docs encourage users to "tell your AI Agent to execute the following command," meaning an AI model may be the one running the command, with no human review of what is being fetched.No new documentation in this PR mentions the supply chain risk or advises pinning a specific version.
Code
# Appears in every new QuickStart and OfficialServerAccess page:
npx add-mcp https://tron-mcp-server.bankofai.io/mcp
npx skills add https://github.com/BofAI/skillsRecommendation
- Pin a specific npm package version in the
npxinvocations (e.g.,npx add-mcp@1.2.3 ...) or document which package owns theadd-mcpandskillsbin aliases. - Add a brief security note alongside each command explaining that it downloads and executes code from npm. Link to the package's npm page / GitHub repo for transparency.
- Alternatively, provide the manual configuration alternative (JSON snippet) as a fallback for security-sensitive users — the same information that was removed in MJ-01.
[MJ-03] BANKOFAISkill.md — "Paste Private Key Directly" Listed as a Valid Option
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security / Documentation |
| File | docs/McpServer-Skills/SKILLS/BANKOFAISkill.md : Lines ~40–50 (new content) |
Description
Under "Where Do I Get These Keys? How Do I Set Them Up?", the new revision presents two credential-setup options for wallet credentials:
- Option 1 (Recommended): Use Agent Wallet — private key encrypted and never exposed.
- Option 2: Paste your private key directly into your system config file (the "notepad method").
Option 2 directly contradicts the security model Agent Wallet is designed to prevent, and contradicts the :::danger blocks in
Agent-Wallet/Intro.md,Agent-Wallet/QuickStart.md, andAgent-Wallet/Developer/SDK-Guide.md. Labeling it "great for power users or quick testing" normalises unsafe key management. A user who reads only the Skill Catalog page and misses the Agent Wallet intro will have no indication that Option 2 is dangerous on anything but a throw-away key.
Code
- **Option 2:** Paste your private key directly into your system config file
(the "notepad method") — great for power users or quick testing.Recommendation
Either remove Option 2 entirely from this page and redirect all users to the Agent Wallet QuickStart, or reframe it with an explicit :::danger admonition (consistent with every other place in the docs where plaintext private key usage is discussed) and restrict it to "fully isolated testnet keys only."
Minor
[MN-01] TRONMCPServer/FAQ.md — Concrete Multi-Server JSON Example Removed Without Replacement
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md : "Can I use multiple TRON MCP Servers simultaneously?" |
Description
The previous answer included a concrete working JSON snippet showing how to define two named MCP server entries simultaneously (mainnet cloud + local testnet). The new answer replaces it with: "Configure both servers with different names in your client's MCP configuration." Without the JSON example, users have no actionable guidance on the correct structure for multi-server configuration.
Recommendation
Restore the JSON example (or a condensed version of it), or link to a dedicated configuration reference that shows the structure. A concrete example is especially valuable in an FAQ.
[MN-02] Agent-Wallet/QuickStart.md — No Windows Tab or Unsupported-OS Notice
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | docs/Agent-Wallet/QuickStart.md : Step 2.1 |
Description
Step 2.1 "Save and Activate the Password" offers two tabs: "Mac Users (Zsh)" and "Linux Users." The Agent Wallet FAQ (same PR) states Windows is not supported. A Windows user landing on this QuickStart will see no guidance for their OS and receive no explanation of why.
Recommendation
Add a third tab "Windows Users" whose content simply states that Windows is not currently supported and directs users to the FAQ (which has the same information). This prevents user confusion and sets accurate expectations early in the flow.
[MN-03] SUNMCPServer/FAQ.md — Double-Quote Inconsistency for TRON_MNEMONIC
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Correctness |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/FAQ.md : Lines ~148–168 |
Description
The PR comprehensively converts
"→'forexportstatements involving sensitive env vars (AGENT_WALLET_PASSWORD,TRON_PRIVATE_KEY,TRON_MNEMONIC). However, in the "Conflicting Wallet Modes" section,TRON_MNEMONICis fixed to single quotes (✅), but in the same blockTRON_PRIVATE_KEYis also fixed (✅). Cross-checking theTRONMCPServer/FAQ.mdequivalent block, the mnemonic fix is correct there too. The fix appears complete in the docs that were changed — this is a note to verify the i18n mirror files received the same treatment consistently (they are mirrored in the diff but worth confirming during QA).
Recommendation
Verify all i18n mirror files under i18n/current/ have the same single-quote fix applied. Consider adding a CI lint rule (e.g., a grep check) to flag export VARIABLE=" patterns in documentation, since this class of error has appeared multiple times across commits.
[MN-04] TRONMCPServer/LocalPrivatizedDeployment.md — Step Numbering Gap
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md |
Description
With Step 4 "Client Configuration" removed, the document now has Steps 1, 2, 3 (Installation), then jumps directly to "Step 4: Verify Connection" (previously Step 5). The verify step was renumbered correctly but the gap (the missing client configuration step) may confuse readers who have bookmarked, followed tutorials, or watched videos referencing the old five-step flow.
Recommendation
Either restore Step 4 content (see MJ-01) or add a brief bridging note explaining that client-specific configuration has moved to a central reference, keeping the prose coherent.
Suggestions
[S-01] docker.yml — workflow_dispatch Comment Wording
File: .github/workflows/docker.yml
Description: The new comment # Intentionally unrestricted — allows manual builds from any branch for flexibility is accurate but slightly informal for a CI file. This is subjective but can be tightened.
Suggestion: # Unrestricted: allows manual workflow triggers from any branch.
[S-02] Agent Wallet SDK Cookbook — fromAddress Hardcoded Placeholder
File: docs/Agent-Wallet/Developer/SDK-Cookbook.md
Description: The TypeScript TRON transfer example uses the string "YOUR_TRON_ADDRESS" as a placeholder for fromAddress and then validates it against wallet.getAddress(). If a user runs the example without substituting the placeholder, the validation will always fail with a confusing error message about address mismatch rather than a clear "you forgot to replace the placeholder" message.
Suggestion: Add a guard at the top of the example that detects the placeholder and throws a helpful error, e.g.:
if (fromAddress === "YOUR_TRON_ADDRESS") {
throw new Error("Replace 'YOUR_TRON_ADDRESS' with your actual TRON wallet address before running.");
}[S-03] SKILLS QuickStart.md — Security Assessment Table Uses Vendor-Opaque Column Headers
File: docs/McpServer-Skills/SKILLS/QuickStart.md
Description: The installation walkthrough shows a security scan table with columns Gen, Socket, and Snyk. These are vendor names/brand abbreviations that end-users unfamiliar with npm security tooling will not recognise. The x402-payment row shows Med and 1 alert which users are instructed to approve without understanding.
Suggestion: Add a brief legend below the table explaining what each column represents and what Med / 1 alert mean (severity level, what kind of vulnerability triggered the flag), so users can make an informed decision before selecting Yes.
Positive Observations
| Area | Observation |
|---|---|
| CI Security | Removed the Debug Docker Hub secrets step that echoed $DOCKERHUB_USERNAME and token length into CI logs — a genuine security improvement that removes information leakage from workflow runs. |
| Env Var Quoting (Systematic) | A dedicated commit (533abed) systematically converts all export VAR="value" to export VAR='value' for passwords and private keys across ~10 files. This prevents shell expansion from silently corrupting special-character passwords — a subtle but real footgun. |
| Non-Revealing Password Check | Replaced echo $AGENT_WALLET_PASSWORD (which would print the password in plain text to the terminal) with `[[ -n "$AGENT_WALLET_PASSWORD" ]] && echo "Password is set" |
| Agent Wallet Documentation Quality | The six new Agent-Wallet files are thorough and well-structured. The threat model in Intro.md is compelling and accurate. SDK-Guide.md clearly delineates "core" vs. "fallback" operating modes with appropriate :::danger admonitions. SDK-Cookbook.md provides complete, runnable end-to-end examples with realistic error handling (signature field check, txID validation). |
| Client-Agnostic Language | Replacing "Claude Desktop" / "Claude Code" / "Cursor" specific references with generic "MCP client" / "AI Agent" terminology in FAQ troubleshooting steps is a good abstraction that extends the docs' useful life across the evolving MCP ecosystem. |
agent-wallet reset Warning |
Both TRON and SUN MCP FAQs now include an explicit warning that agent-wallet reset wipes all wallets and keys, with a prompt to back up mnemonics before proceeding — a meaningful safety improvement over the old rm -rf ~/.agent-wallet/ raw instruction. |
| Security Warnings Consistency | The Agent Wallet documentation consistently applies :::caution for the env-var quoting pitfall, :::danger for static injection mode / raw_secret wallet type, and plain prose callouts elsewhere. The severity scale is appropriate and used consistently. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 5 | 4 | 1 | 0 | Step numbering gap in LocalPrivatizedDeployment |
| Security | 8 | 5 | 3 | 0 | npx supply chain, plaintext key as Option 2, single-quote audit of i18n |
| Performance | 3 | 3 | 0 | 0 | No performance-sensitive code changed |
| Code Quality | 6 | 5 | 1 | 0 | Removed content not replaced (MJ-01) |
| Testing | 4 | 4 | 0 | 0 | Docs repo — no functional tests required |
| Documentation | 8 | 6 | 2 | 0 | MJ-01 (incomplete config section), MN-01 (missing JSON example) |
| Compatibility | 3 | 2 | 1 | 0 | Windows unsupported but not surfaced in QuickStart (MN-02) |
| Observability | 2 | 2 | 0 | 0 | CI debug step removed; no new observability gaps |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between main and update-mcp-server. Runtime behavior, integration testing, and deployment impact are not covered. All line number references are approximate relative to the diffed content.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-24
Code Review ReportProject: BofAI Docs (x402-tron documentation site) PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR is primarily a documentation update adding a comprehensive new Agent Wallet section (Intro, QuickStart, FAQ, CLI Reference, SDK Guide, and SDK Cookbook) and revising the MCP Server and Skills documentation to be client-agnostic (replacing "Claude Desktop" references with generic "MCP client" terminology). The PR also removes a credentials-leaking debug step from the CI workflow and improves security messaging throughout the docs. The code changes are purely documentation and configuration (sidebars, workflow YAML, package.json version bump) — there is no application logic being changed. The most significant security concern is that the The newly added Agent Wallet documentation is detailed, well-structured, and demonstrates strong security awareness — it consistently warns users about private-key exposure risks, recommends encrypted storage over plaintext environment variables, and provides clear operational guidance. The documentation improvements across the MCP server sections remove Claude Desktop-specific configuration examples, replacing them with a generic Change SummaryCI/CD Workflow Update
Purpose: Align the Docker build workflow with the new working branch and remove a security anti-pattern (echoing credentials in CI logs). New Agent Wallet Documentation
Purpose: Introduce a new documentation section for the Agent Wallet product, covering all user types from beginners to SDK developers. MCP Server and Skills Documentation Revisions
Purpose: Make documentation client-agnostic (not locked to Claude Desktop), simplify user-facing language, and align MCP installation guidance to use the new Sidebar and Configuration
Purpose: Wire the new Agent Wallet documentation into the site navigation and bump the package version. Detailed Findings[MJ-01]
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security |
| File | .github/workflows/docker.yml : Line 12 |
Description
The
workflow_dispatchtrigger, now annotated as "intentionally unrestricted," combined withpush: ${{ github.event_name != 'pull_request' }}, means any user with write access to the repository can manually trigger this workflow from any arbitrary branch and push the resulting image to Docker Hub under thebankofai/docs:testtag. The comment in the diff says this is intentional for "flexibility," but it creates a risk: a compromised or careless contributor could publish a modified Docker image without a PR review.
Code
workflow_dispatch: # Intentionally unrestricted — allows manual builds from any branch for flexibilityRecommendation
Consider adding a branch restriction to workflow_dispatch or, at minimum, document this risk in a team runbook. If unrestricted dispatch is truly desired, consider removing the push: true step for manual dispatches or requiring a specific input parameter confirming intent:
workflow_dispatch:
inputs:
confirm_push:
description: 'Type "push" to confirm pushing to Docker Hub'
required: true[MJ-02] SDK Cookbook Contains Hardcoded Mainnet API URL with No Rate-Limit or Auth Warning
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness / Docs |
| File | docs/Agent-Wallet/Developer/SDK-Cookbook.md : Lines 55-57 |
Description
The TRON Transfer example hardcodes
https://nile.trongrid.ioas a code constant with a comment saying "change tohttps://api.trongrid.iofor mainnet." The mainnet endpoint requires a TronGrid API key for production use (rate limit is 100k/day without one). The cookbook provides no guidance on injecting the API key, nor does it note that the mainnet endpoint will throttle unauthenticated requests. Users copying this directly into production will silently encounter 429 errors at scale.
Code
const TRONGRID_API = "https://nile.trongrid.io"; // Nile testnet; change to https://api.trongrid.io for mainnetRecommendation
Add a note or environment variable pattern for the API key, and reference the TronGrid API key documentation:
const TRONGRID_API = process.env.TRONGRID_API || "https://nile.trongrid.io";
// For mainnet: use https://api.trongrid.io
// Add TRONGRID-API-KEY header if you have a TronGrid API key (strongly recommended for production)Or add a caution admonition in the surrounding Markdown explaining the rate-limit situation for mainnet.
[MJ-03] Removal of Client-Specific JSON Configuration Examples Reduces Discoverability
| Property | Value |
|---|---|
| Severity | Major |
| Category | Docs |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md, docs/McpServer-Skills/MCP/SUNMCPServer/OfficialServerAccess.md, docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md, docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md |
Description
Multiple files previously contained detailed, per-client (Claude Desktop, Claude Code, Cursor, Generic HTTP) JSON configuration examples in tabbed format. This PR removes all of them, replacing them with a single
npx add-mcpornpx skills addcommand flow. While the new installer approach is simpler for supported clients, it gives users no fallback when theadd-mcptool does not support their client or they need to configure manually. The removal of the Claude Codeclaude mcp add ...command and the.mcp.json/.cursor/mcp.jsonexamples is a notable regression for power users and developers who need precise control.
Code
-<Tabs>
-<TabItem value="Claude Desktop" label="Claude Desktop">
-...explicit JSON config examples...
-</TabItem>
-<TabItem value="Claude Code" label="Claude Code">
- claude mcp add --transport http mcp-server-tron https://tron-mcp-server.bankofai.io/mcp
-</TabItem>Recommendation
Retain the per-client manual configuration as a collapsible <details> section or an "Advanced / Manual Configuration" subsection so that: (a) the happy path remains simple via npx add-mcp, and (b) users on unsupported clients or those needing manual control still have authoritative reference material.
[MN-01] AGENT_WALLET_DIR Set with Unexpanded Tilde in Environment Variable
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness / Docs |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/LocalPrivatizedDeployment.md : Line 80 |
Description
The diff changes the
AGENT_WALLET_DIRexample from"~/.agent-wallet"(which does not expand in all contexts when used as an env var value set viaexport) to"$HOME/.agent-wallet". This is a correct improvement. However, the fix is applied inconsistently — some files in the PR still show the old"~/.agent-wallet"form inexportstatements. For example, indocs/Agent-Wallet/Developer/CLI-Reference.md,--dir /Volumes/MyUSB/agent-walletis used (fine), but earlier examples in other new docs still show~/.agent-walletas a hard path in comments rather than with$HOME.
Code
# Fixed correctly in SUNMCPServer/LocalPrivatizedDeployment.md:
export AGENT_WALLET_DIR="$HOME/.agent-wallet"
# But Agent-Wallet FAQ.md still mentions:
ls ~/.agent-wallet/Recommendation
Audit all uses of ~/.agent-wallet in the new documentation. Shell expansion of ~ works in interactive shells but is not guaranteed when a value is read programmatically (e.g., by an MCP server reading the env var). For env var declarations, consistently use $HOME/.agent-wallet.
[MN-02] Broken or Unverifiable Anchor Links to CLI Reference Section
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness / Docs |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/FAQ.md : Line 143, docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md : Line 95 |
Description
Multiple places in the FAQ documents link to
../../../Agent-Wallet/Developer/CLI-Reference#agent-wallet-reset-reset-all-data. The anchor fragment#agent-wallet-reset-reset-all-datais a generated Markdown heading slug. The actual heading inCLI-Reference.mdis### \agent-wallet reset` (Reset All Data). Docusaurus generates heading slugs from the text, not the code, and backtick code spans affect slug generation. The actual generated anchor is likely#agent-wallet-reset-reset-all-datawhich matches, but the parentheses in the heading(Reset All Data)` get stripped. This should be verified — if the anchor does not match, the links will silently point to the top of the page.
Code
Run `agent-wallet reset` to wipe and start over — see [CLI Reference → Reset](../../../Agent-Wallet/Developer/CLI-Reference#agent-wallet-reset-reset-all-data)Recommendation
Verify all cross-document anchor links using docusaurus build or a link checker. If the slug does not match, use an explicit {#custom-id} on the target heading:
### `agent-wallet reset` (Reset All Data) {#agent-wallet-reset}Then update links to #agent-wallet-reset.
[MN-03] raw_secret Wallet Type Documented Without Sufficient CLI Discoverability Warning
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Security / Docs |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Lines 18-26 |
Description
The CLI Reference documents
raw_secretas a wallet type that "stores your private key unencrypted on disk." It includes a:::dangeradmonition advising against it for real funds. However, the QuickStart (docs/Agent-Wallet/QuickStart.md) shows an interactive wizard screenshot where the first option presented islocal_secure— without mentioning thatraw_secretexists at that stage. This is good. However, the CLI Reference's danger block does not tell users how to check whether an existing wallet israw_secretvs.local_secure, which could be important for users who may have accidentally created araw_secretwallet in the past.
Code
:::danger Avoid the `raw_secret` wallet type for real funds
The interactive wizard may offer a `raw_secret` option. This type stores your private key **unencrypted** on disk...
:::Recommendation
Add a note explaining how to inspect wallet type:
agent-wallet inspect <wallet-id> # shows wallet type in outputOr note that agent-wallet list shows the type column.
[MN-04] package.json Version Bump Is Undocumented
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Docs / Quality |
| File | package.json : Line 3 |
Description
The version is bumped from
1.2.3to1.2.4with no changelog entry or release notes. For a documentation site, this may be inconsequential, but if the version is used to track deployed releases or is surfaced anywhere in the UI, a CHANGELOG or release note would help maintainers understand what changed.
Code
- "version": "1.2.3",
+ "version": "1.2.4",Recommendation
Maintain a CHANGELOG.md or at minimum document the version bump reason in the PR description. If the docs site version is purely internal bookkeeping, consider noting that in the package.json description field.
[MN-05] workflow_dispatch Comment Uses an Em Dash That May Cause YAML Parsing Issues on Older Tools
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness |
| File | .github/workflows/docker.yml : Line 12 |
Description
The added comment contains a Unicode em dash character (—). While modern YAML parsers handle this without issue, some older CI tooling and YAML linters may flag non-ASCII characters in comments as warnings. This is a low-risk concern but worth noting for CI hygiene.
Code
workflow_dispatch: # Intentionally unrestricted — allows manual builds from any branch for flexibilityRecommendation
Replace the em dash with a regular hyphen for maximum compatibility:
workflow_dispatch: # Intentionally unrestricted - allows manual builds from any branch for flexibility[S-01] Security Improvement: echo $AGENT_WALLET_PASSWORD Pattern Correctly Replaced
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Security / Quality |
| File | docs/McpServer-Skills/MCP/SUNMCPServer/FAQ.md : Lines 135-138 |
Description
The PR correctly replaces the dangerous
echo $AGENT_WALLET_PASSWORD(which would print the password to terminal/logs) with a safe existence-check pattern. This is a good fix. Consider documenting this pattern in a shared security style guide so future doc contributors know to use it.
Code
# New (correct) pattern:
[[ -n "$AGENT_WALLET_PASSWORD" ]] && echo "Password is set" || echo "Password NOT set"Recommendation
Add a note to the repository's contributing guide or documentation style guide: "When showing how to verify an environment variable is set, never use echo $SECRET_VAR. Use [[ -n "$SECRET_VAR" ]] && echo 'set' || echo 'not set' instead."
[S-02] npx add-mcp Tool Is Not a Well-Known Standard — No Validation That It Exists
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Correctness / Docs |
| File | docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md : Lines 47-52 |
Description
The PR replaces all client-specific MCP configuration instructions with
npx add-mcp <url> -y. This tool (add-mcp) is not an officially recognized npm package in the broader MCP ecosystem and appears to be a project-specific or community tool. If the package is unmaintained, renamed, or removed from npm, all these documentation links become broken with no fallback instructions. Similarly,npx skills addused in the SKILLS QuickStart is project-specific. These tools' reliability and longevity should be validated.
Code
npx add-mcp https://tron-mcp-server.bankofai.io/mcp -yRecommendation
- Explicitly link to the npm page or GitHub repo for
add-mcpso users can verify it. - Keep the manual JSON configuration as a fallback (see MJ-03).
- Consider pinning a version:
npx add-mcp@<version>to avoid unexpected breaking changes.
[S-03] SKILLS QuickStart Shows x402-payment with "Med" Security Alert Without Explanation
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Security / Docs |
| File | docs/McpServer-Skills/SKILLS/QuickStart.md : Lines 110-123 |
Description
The installation walkthrough shows a security assessment table where
x402-paymenthasMedrisk ratings from both Gen and Snyk, plus "1 alert." The guide then simply shows the user selecting "Yes" to proceed without explaining what the alerts are or how to evaluate them. Users following this walkthrough will develop a habit of clicking past security alerts without understanding them.
Code
│ x402-payment Med 1 alert Med │
...
◇ Proceed with installation?
│ Yes
Recommendation
Add a note explaining what Med means in this context and that users should review the specific alerts. For example: "The Med rating for x402-payment is typical for payment-related packages that handle cryptographic operations. Click the alert link to review the specific finding before proceeding."
[S-04] Openclaw QuickStart Uses Gamified "Level" Framing That May Obscure Critical Setup Steps
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Docs / Quality |
| File | docs/Openclaw-extension/QuickStart.md : Lines 28-60 |
Description
The new QuickStart uses a gaming metaphor ("Level 1", "Level 2", etc.) to describe installation steps. While this is engaging, critical steps like AgentWallet configuration are described as "just close your eyes and press Enter all the way through — the default values are perfectly fine." This may lead users to skip reviewing important security configuration without understanding what they are accepting.
Code
If you're a beginner, just close your eyes and press Enter all the way through — the default values are perfectly fine.Recommendation
Retain the approachable tone but ensure security-relevant steps are explicitly identified. For example: "For beginners: the defaults are safe to accept. The one exception is your master password — make sure to copy it when shown." Avoid "close your eyes" phrasing for security-sensitive steps.
[S-05] zh-Hans Sidebar Uses Hardcoded Chinese String Instead of Translation Key
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Quality / Consistency |
| File | i18n/zh-Hans/docusaurus-plugin-content-docs/current/sidebars.js : Line 159 |
Description
The Chinese sidebar hardcodes the label
'进一步探索'directly in the sidebar JS file rather than using a translation key (as is done for other categories). The English sidebar uses'Explore Further'which Docusaurus resolves viacurrent.json. The Chinese sidebar could also use'Explore Further'as the key and rely on the translation file, keeping the pattern consistent.
Code
// In i18n/zh-Hans/...sidebars.js:
label: '进一步探索', // hardcoded Chinese string
// vs. English sidebars.js:
label: 'Explore Further', // resolved via current.json translation keyRecommendation
Use the same label key 'Explore Further' in the zh-Hans sidebar.js as well and rely on the current.json translation entry (which was also added in this PR: "sidebar.docsSidebar.category.Explore Further": { "message": "进一步探索", ... }). This makes future label changes consistent across both files.
[S-06] Commit History Contains Many Undescriptive "fix" Messages
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Quality / Maintainability |
| File | Git commit history |
Description
17 of the 47 commits are titled "fix" with no additional context, and several others have misspellings ("fiix", "fxi"). While this does not affect the code itself, it makes bisecting and auditing history extremely difficult.
Code
47b8030 fxi
224d32d fiix
822bdae fix
3b87508 fix
824c0d7 fix
...
Recommendation
Adopt a commit convention (e.g., Conventional Commits: docs:, fix:, feat:) and consider squashing fixup commits before merging to main to maintain a clean, navigable history.
Positive Observations
| Area | Observation |
|---|---|
| Security: CI credential hygiene | The debug step that echoed Docker Hub credentials (echo "username=[$DOCKERHUB_USERNAME]") was correctly removed. This was a clear security anti-pattern. |
| Security: env var quoting | Multiple locations correctly changed double-quoted env var assignments ("your_password") to single-quoted ('your_password') to prevent shell expansion of special characters in passwords. |
| Security: password existence check | Replaced echo $AGENT_WALLET_PASSWORD (which reveals the password) with [[ -n "$AGENT_WALLET_PASSWORD" ]] (which only checks existence) — a meaningful security improvement in user-facing docs. |
| Documentation depth | The new Agent Wallet documentation is comprehensive, covering beginner to developer audiences with consistent cross-references, dual TypeScript/Python examples, and well-structured admonition blocks. |
| Security documentation | The Agent Wallet introduction clearly explains the two-lock security model, the threat model (dependency chain poisoning, accidental log exposure, git history), and how local_secure mitigates each. |
| Error handling examples | The SDK Guide includes explicit error handling patterns with typed exceptions (WalletNotFoundError, DecryptionError, SigningError), which is developer-friendly and promotes defensive coding. |
| Client-agnostic improvements | Replacing "Claude Desktop" specificity with generic "MCP client" language throughout is a meaningful improvement to the longevity of the documentation. |
| Inline warnings on dangerous operations | The agent-wallet reset command is consistently accompanied by strong warnings about irreversibility and the need to back up funds first. |
| Chinese translation coverage | All new English documentation has corresponding Chinese translations, maintaining full i18n coverage. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 8 | 6 | 2 | 0 | Anchor link correctness unverified (MN-02); ~ vs $HOME inconsistency (MN-01) |
| Security | 10 | 8 | 2 | 0 | workflow_dispatch unrestricted push (MJ-01); echo $PASSWORD patterns fixed; credential debug step removed |
| Performance | 3 | 3 | 0 | 0 | No performance-affecting code; documentation only |
| Code Quality | 8 | 5 | 3 | 0 | Undescriptive commit messages (S-06); zh-Hans hardcoded label (S-05); missing fallback config examples (MJ-03) |
| Testing | 4 | 2 | 0 | 2 | No tests applicable to documentation; workflow CI tests are not modified |
| Documentation | 12 | 11 | 1 | 0 | SDK Cookbook lacks mainnet API key guidance (MJ-02) |
| Compatibility | 5 | 4 | 1 | 0 | Removal of per-client JSON examples is a regression for unsupported clients (MJ-03) |
| Observability | 3 | 3 | 0 | 0 | Error types documented; no application code changed |
Disclaimer
This is an automated code review. It supplements but does not replace human review.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-24
Code Review ReportProject: PR OverviewBranch Information
Commit History (Recent)
Review SummaryVerdict
Findings at a Glance
SummaryThis PR is a large documentation update for the Overall the documentation quality is high: security warnings are prominently placed using Docusaurus admonition blocks ( Change Summary1. CI/CD Pipeline —
|
| File | Change Type | Description |
|---|---|---|
.github/workflows/docker.yml |
Modified | Replace ai-bankofai-patch-1 branch references with update-mcp-server; remove debug step that printed Docker Hub credentials; add comment on workflow_dispatch |
Purpose: Keep the build pipeline aligned with the new working branch, and remove a pre-existing security concern where the CI step echoed DOCKERHUB_USERNAME and the length of DOCKERHUB_TOKEN to the GitHub Actions log.
2. New Agent Wallet Documentation (English)
| File | Change Type | Description |
|---|---|---|
docs/Agent-Wallet/Intro.md |
Added | Overview of Agent Wallet, supported chains, architecture, links |
docs/Agent-Wallet/QuickStart.md |
Added | Step-by-step guide: install Node.js, create wallet, configure password, call from OpenClaw |
docs/Agent-Wallet/FAQ.md |
Added | Security model, encryption details, troubleshooting 12 common questions |
docs/Agent-Wallet/Developer/CLI-Reference.md |
Added | Full CLI command reference including automation, dangerous operations, shell script example |
docs/Agent-Wallet/Developer/SDK-Guide.md |
Added | SDK integration guide with environment variable configuration, supported networks, error handling |
docs/Agent-Wallet/Developer/SDK-Cookbook.md |
Added | End-to-end code examples: TRON TRX transfer, BSC BNB transfer, x402 PaymentPermit signing |
Purpose: Introduces a complete documentation section for the Agent Wallet product that was previously undocumented.
3. New Agent Wallet Documentation (Chinese — i18n/zh-Hans)
| File | Change Type | Description |
|---|---|---|
i18n/zh-Hans/.../Agent-Wallet/Intro.md |
Added | Chinese translation of Intro |
i18n/zh-Hans/.../Agent-Wallet/QuickStart.md |
Added | Chinese translation of QuickStart |
i18n/zh-Hans/.../Agent-Wallet/FAQ.md |
Added | Chinese translation of FAQ |
i18n/zh-Hans/.../Agent-Wallet/Developer/CLI-Reference.md |
Added | Chinese translation of CLI Reference |
i18n/zh-Hans/.../Agent-Wallet/Developer/SDK-Guide.md |
Added | Chinese translation of SDK Guide |
i18n/zh-Hans/.../Agent-Wallet/Developer/SDK-Cookbook.md |
Added | Chinese translation of SDK Cookbook |
i18n/zh-Hans/.../current.json |
Modified | Added "Explore Further" → "进一步探索" sidebar label translation |
Purpose: Provides full Chinese localisation parity for the new Agent Wallet section.
4. MCP Server Documentation Rewrite
| File | Change Type | Description |
|---|---|---|
docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md |
Modified | Replaces multi-tab manual JSON config with npx add-mcp auto-install flow |
docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md |
Modified | Simplified installation steps, removed verbose tab-based config blocks |
docs/McpServer-Skills/MCP/TRONMCPServer/QuickStart.md |
Modified | Updated to reference new auto-install command |
docs/McpServer-Skills/MCP/TRONMCPServer/Intro.md |
Modified | Minor wording updates |
docs/McpServer-Skills/MCP/TRONMCPServer/FAQ.md |
Modified | Updated FAQ entries |
docs/McpServer-Skills/MCP/TRONMCPServer/ToolList.md |
Modified | Changed list_wallets/select_wallet descriptions to link to Agent Wallet docs |
docs/McpServer-Skills/MCP/SUNMCPServer/*.md |
Modified | Parallel updates to SUN MCP Server docs |
docs/McpServer-Skills/MCP/Intro.md |
Modified | Minor update |
All corresponding i18n/zh-Hans/…/MCP/… |
Modified | Chinese translations updated in parallel |
Purpose: Modernises the MCP Server setup experience from manual copy-paste JSON config to a one-line npx add-mcp command. This is a significant UX improvement.
5. SKILLS Documentation Updates
| File | Change Type | Description |
|---|---|---|
docs/McpServer-Skills/SKILLS/QuickStart.md |
Added | New quick start guide using npx skills add command |
docs/McpServer-Skills/SKILLS/Intro.md |
Modified | Revised introduction, added references to new QuickStart |
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md |
Modified | Renamed section, condensed skill catalog |
docs/McpServer-Skills/SKILLS/Faq.md |
Modified | Updated FAQ entries |
Purpose: Brings SKILLS documentation to the same structural standard as MCP Server docs.
6. Openclaw Extension Documentation
| File | Change Type | Description |
|---|---|---|
docs/Openclaw-extension/Intro.md |
Modified | Wording updates, additional context |
docs/Openclaw-extension/QuickStart.md |
Modified | Revised installation steps |
docs/Openclaw-extension/FAQ.md |
Modified | Expanded FAQ |
Purpose: General documentation refresh for the Openclaw Extension section.
7. Sidebar and Package Updates
| File | Change Type | Description |
|---|---|---|
sidebars.js |
Modified | Added "Agent Wallet" category; added SKILLS/QuickStart entry |
i18n/zh-Hans/.../sidebars.js |
Modified | Added "Agent Wallet" category (Chinese version) |
package.json |
Modified | Version bump 1.2.3 → 1.2.4 |
Purpose: Registers all new documentation pages in the navigation sidebar.
Detailed Findings
Major
[MJ-01] Unrestricted workflow_dispatch Allows Any Branch to Push to Docker Hub
| Property | Value |
|---|---|
| Severity | Major |
| Category | Security / CI-CD |
| File | .github/workflows/docker.yml : Lines 13–14 |
Description
The workflow_dispatch trigger has no branch restriction, which means any user with repository write access can manually trigger a Docker build from any branch — including unreviewed feature branches — and push the resulting image to Docker Hub (because push: ${{ github.event_name != 'pull_request' }} evaluates to true for workflow_dispatch events).
While the image is tagged only as test, the push still overwrites the test tag on Docker Hub. If consumers or CI pipelines depend on that tag for staging or validation environments, an accidental or malicious manual dispatch from an unstable branch could introduce a bad image.
The added comment acknowledges this was intentional ("allows manual builds from any branch for flexibility"), but the risk of overwriting the shared test tag is not documented.
Code
- # Before:
- workflow_dispatch:
+ # After:
+ workflow_dispatch: # Intentionally unrestricted — allows manual builds from any branch for flexibilityRecommendation
Add an environment gate or explicit if condition to the push step for workflow_dispatch events, so that manual dispatches on non-main/non-master branches build but do not push to the registry:
- name: Build and push Docker image
uses: docker/build-push-action@v6
with:
context: .
# Only push on push events to main/master/tags, or on workflow_dispatch from main/master
push: ${{ github.event_name == 'push' || (github.event_name == 'workflow_dispatch' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master')) }}Alternatively, add a note in the comment explaining which tag test maps to and whether consumers are expected to depend on it.
Minor
[MN-01] Repeated Weak Example Password May Confuse Security-Conscious Users
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / Security |
| File | docs/Agent-Wallet/Developer/CLI-Reference.md : Lines 29, 44, 81, 88, 170, 185, 216, 238 (and equivalents in SDK-Guide, QuickStart, zh-Hans translations) |
Description
The password Abc12345! is used as the example value throughout all Agent Wallet documentation (CLI Reference, SDK Guide, QuickStart, SDK Cookbook). This exact string meets the tool's stated password requirements (8+ chars, upper, lower, digit, special), which may lead less-experienced users to copy it verbatim as their actual master password — particularly in quick-start or automation setups.
The documentation does contain :::caution Shell history risk and :::danger admonitions, but these appear after the command examples and may not be read by users who copy commands directly.
Code
agent-wallet start -p Abc12345!
export AGENT_WALLET_PASSWORD='Abc12345!'
agent-wallet sign msg "Hello" -n tron -p 'Abc12345!' --save-runtime-secretsRecommendation
Replace the literal Abc12345! with a clearly placeholder-style value such as <YourPassword123!> or 'YOUR_MASTER_PASSWORD'. This makes it immediately obvious that the user must substitute their own credential:
agent-wallet start -p '<YourPassword123!>'
export AGENT_WALLET_PASSWORD='<YourPassword123!>'If a concrete-looking example is preferred for visual clarity, prepend a code comment:
# Replace the password below with your own — do NOT use this example value
export AGENT_WALLET_PASSWORD='Abc12345!'[MN-02] i18n Translation Key Missing for "Agent Wallet" Sidebar Category Label
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation / Compatibility |
| File | i18n/zh-Hans/docusaurus-plugin-content-docs/current.json : End of file |
Description
The PR correctly adds the "Explore Further" → "进一步探索" translation to current.json, but it does not add a translation key for the parent "Agent Wallet" sidebar category label. The zh-Hans sidebars.js currently hardcodes the English string 'Agent Wallet' directly in the label field:
{
type: 'category',
label: 'Agent Wallet', // ← not translated
...
}This is inconsistent with other translated sections (e.g., 'Openclaw 扩展插件', 'MCP 服务器') which use Chinese labels directly, and differs from how the "Explore Further" sub-category is translated via current.json.
Code (i18n/zh-Hans/docusaurus-plugin-content-docs/current/sidebars.js)
{
type: 'category',
label: 'Agent Wallet', // should be '智能体钱包' or similar
collapsed: false,
items: [ ... ]
}Recommendation
Either:
- Change the
labelin zh-Hanssidebars.jsto the Chinese equivalent (e.g.,'智能体钱包'), or - Add a translation key in
current.json:
"sidebar.docsSidebar.category.Agent Wallet": {
"message": "智能体钱包",
"description": "The label for category Agent Wallet in sidebar docsSidebar"
}and use translationId in the sidebars entry.
[MN-03] Sidebars Reference BSC MCP Server Pages Not Included in This PR
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness / Documentation |
| File | sidebars.js : BSC MCP Server category block (lines ~115-130 in updated file) |
Description
The updated sidebars.js contains a sidebar category for BSC MCP Server that references three doc IDs:
{
type: 'category',
label: 'BSC MCP Server',
collapsed: true,
items: [
'McpServer-Skills/MCP/BSCMCPServer/Intro',
'McpServer-Skills/MCP/BSCMCPServer/Features',
'McpServer-Skills/MCP/BSCMCPServer/Installation',
],
},None of these files (BSCMCPServer/Intro.md, BSCMCPServer/Features.md, BSCMCPServer/Installation.md) are present in the diff or otherwise added in this PR. If the Docusaurus build runs with docs.onBrokenLinks: 'throw' (the default), this will cause a build failure. If set to 'warn', broken links will silently appear in the built site.
Recommendation
Either:
- Add the missing BSC MCP Server documentation files in this PR (or a follow-up), or
- Remove the BSC MCP Server sidebar entry until the corresponding pages are ready:
// TODO: Add BSC MCP Server docs before re-enabling this sidebar entry
// {
// type: 'category',
// label: 'BSC MCP Server',
// ...
// }Suggestions
[S-01] Run a Broken Link Check Before Merging
File: Multiple MCP Server docs (e.g., docs/McpServer-Skills/MCP/TRONMCPServer/ToolList.md, docs/McpServer-Skills/MCP/TRONMCPServer/OfficialServerAccess.md)
Description: The MCP Server documentation introduces a number of new cross-document links pointing to the new Agent-Wallet/Intro pages. While the target pages exist in this PR, a Docusaurus build should be run to validate that no links are broken — especially given [MN-03] above with the BSC MCP Server references.
Suggestion: Add a CI step or pre-merge gate:
yarn build # Will throw on broken links by default in DocusaurusThis ensures any broken internal links are caught automatically.
[S-02] Document the test Docker Tag Semantics in docker.yml
File: .github/workflows/docker.yml
Description: The Docker image is always tagged test regardless of the source branch. There is no indication in the workflow of who consumes this tag or what contract it represents. Related to [MJ-01], callers relying on the test tag for CI/CD validation could be silently broken by accidental or premature pushes.
Suggestion: Add a comment block near the tag configuration clarifying the expected usage lifecycle of the test tag and whether it represents a stable preview or a volatile snapshot.
[S-03] Consider Adding a CHANGELOG or Release Notes Entry
File: package.json (version 1.2.4), documentation root
Description: The version bump to 1.2.4 is unaccompanied by any CHANGELOG.md or release note entry. For a documentation project, a brief changelog entry makes it easier to communicate what new content was added in each release (e.g., for internal stakeholders, link-sharing, or versioned docs).
Suggestion: Add a CHANGELOG.md entry (or update an existing one) noting the addition of the Agent Wallet documentation section and the MCP Server auto-install flow changes.
Positive Observations
| Area | Observation |
|---|---|
| Security Fix | The removal of the "Debug Docker Hub secrets" step (which was printing DOCKERHUB_USERNAME and token length to CI logs) is an excellent proactive security improvement. |
| Security Documentation | New Agent Wallet docs consistently use :::danger and :::caution admonition blocks to call out high-risk operations (e.g., raw_secret mode, inline -p flag, --save-runtime-secrets). The coverage is thorough and appropriately placed. |
| Dual-Language Code Examples | All SDK Cookbook examples are provided in both TypeScript and Python with tab switching — a best practice for a multi-language developer audience. |
| Validated Code Patterns | The SDK Cookbook includes runtime validation checks after each API call (e.g., checking for missing txID and signature fields after TronGrid calls, JSON parse error handling). |
| CI Best Practice | The new CI/CD example in CLI-Reference.md correctly uses ${{ secrets.AGENT_WALLET_PASSWORD }} and explicitly warns against hardcoding passwords in workflow files. |
| Non-Breaking Navigation | The new Agent-Wallet sidebar category is inserted between existing sections without disrupting the existing navigation order. |
| Shell Quoting Guidance | The documentation consistently warns about using single quotes vs. double quotes for passwords with special characters in shell environments — a common source of subtle bugs. |
| Progressive Disclosure | Quick Start pages cover the minimal path with links to deeper references (CLI Reference, SDK Guide, SDK Cookbook, FAQ), following a good progressive-disclosure documentation pattern. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 5 | 4 | 1 | - | MN-03: broken sidebar references to BSC MCP Server pages |
| Security | 6 | 5 | 1 | - | MJ-01: unrestricted workflow_dispatch can push to Docker Hub from any branch |
| Performance | 3 | - | - | 3 | No runtime code changed; N/A for docs-only changes |
| Code Quality | 5 | 4 | 1 | - | MN-01: example password Abc12345! reused ubiquitously |
| Testing | 3 | - | - | 3 | No application tests; docs site relies on Docusaurus build validation |
| Documentation | 6 | 5 | 1 | - | MN-02: missing i18n translation key for "Agent Wallet" category label |
| Compatibility | 3 | 3 | - | - | sidebars.js structure correct, package.json version bump appropriate |
| Observability | 2 | 2 | - | - | Debug secrets step correctly removed; remaining logging is appropriate |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between remotes/origin/main and remotes/origin/update-mcp-server. Runtime behavior, integration testing, and deployment impact are not covered. Documentation accuracy (e.g., correctness of blockchain API URLs, SDK method signatures) has not been independently verified against live services.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-24
2 participants
No description provided.