Skip to content

Update mcp server#37

Merged
Will-Guan merged 11 commits intomainfrom
update-mcp-server
Apr 1, 2026
Merged

Update mcp server#37
Will-Guan merged 11 commits intomainfrom
update-mcp-server

Conversation

@jizhen181-dot
Copy link
Copy Markdown
Collaborator

Enriched the content of openclaw-extension

@github-actions
Copy link
Copy Markdown

Code Review Report

Project: BofAI / openclaw-extension (Docusaurus Docs Site)
PR: mainupdate-mcp-server
Review Date: 2026-03-25
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)


PR Overview

Branch Information

Property Value
From Branch main
To Branch update-mcp-server
Commits 4 (930728f, ecfa0c5, fc7883d, 763194a)
Files Changed 6
Lines Added +548
Lines Removed -80

Commit History

Hash Message
930728f add openclaw extension
ecfa0c5 Configure "AI's Personal Vault AgentWallet"
fc7883d update openclaw-extension
763194a Merge pull request #36 from BofAI/update-mcp-server

Review Summary

Verdict

Verdict: Request Changes

Findings at a Glance

Critical Major Minor Suggestion
Count 0 2 3 3

Summary

This PR is a documentation-only update to the OpenClaw Extension installation guides for both the English and Chinese (zh-Hans) Docusaurus sites. The core changes are: (1) dropping Python 3 as a prerequisite in favour of Git, (2) lowering the Node.js minimum from v20 to v18, (3) promoting AgentWallet from a silent background step to a first-class "third core weapon", and (4) significantly expanding the QuickStart guide with verbatim installer UI output, per-server configuration walkthroughs, a skill security-assessment table, a Configuration File Reference, and a new Security Tips section.

The PR represents a meaningful quality improvement in clarity and user guidance. However, two major issues require attention before merge: a curl | bash remote-code-execution pattern is used in a user-facing command that is not warned about, and a URL-breaking change to the BANK OF AI API base URL will silently break existing user configurations without a migration note. Several minor and suggestion-level issues are also documented below.


Change Summary

1. Prerequisite List Update (FAQ + QuickStart – EN & ZH)

File Change Type Description
docs/Openclaw-extension/FAQ.md Modified Replaced "command not found: python3" section with "command not found: git"
docs/Openclaw-extension/QuickStart.md Modified Removed Python 3 from prerequisites; lowered Node.js requirement from v20 to v18
i18n/zh-Hans/.../FAQ.md Modified Same as above, in Chinese
i18n/zh-Hans/.../QuickStart.md Modified Same as above, in Chinese

Purpose: The installer was refactored to eliminate the Python 3 dependency; Git was already listed but its FAQ entry was missing.


2. Intro Architecture Rewrite (Intro – EN & ZH)

File Change Type Description
docs/Openclaw-extension/Intro.md Modified Rewrote intro copy; promoted AgentWallet to "Weapon 1" making total weapons 3 instead of 2
i18n/zh-Hans/.../Intro.md Modified Same, in Chinese

Purpose: Align the conceptual architecture described in the Intro with the actual 3-step installer flow (Vault → Toolbox → Skills).


3. QuickStart Detailed Walkthrough Expansion (QuickStart – EN & ZH)

File Change Type Description
docs/Openclaw-extension/QuickStart.md Modified +237 lines; added verbatim installer prompts/output, per-server config sections, skill table with risk scores, Installation Complete banner, Config File Reference table, Security Tips section
i18n/zh-Hans/.../QuickStart.md Modified Same, in Chinese

Purpose: Replace terse bullet-list instructions with a fully guided walkthrough matching the real installer experience.


4. BANK OF AI API Base URL Change (QuickStart – EN & ZH)

File Change Type Description
docs/Openclaw-extension/QuickStart.md Modified base_url changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/
i18n/zh-Hans/.../QuickStart.md Modified Same, in Chinese

Purpose: Reflects a change in the BANK OF AI API endpoint.


Detailed Findings


Major

[MJ-01] Breaking API URL Change Has No Migration Notice

Property Value
Severity Major
Category Documentation / Correctness
File docs/Openclaw-extension/QuickStart.md : Lines 329–331 (EN); i18n/zh-Hans/.../QuickStart.md : equivalent lines

Description

The manual config snippet for BANK OF AI changes base_url from https://api.bankofai.io/v1/ to https://chat.bankofai.io/. Existing users who have already run the old installer and generated ~/.mcporter/bankofai-config.json with the old URL will silently fail to connect to BANK OF AI services after reading the new docs — or, worse, will copy the new snippet without realising their old file still points to the dead endpoint. There is no callout, :::caution block, or note that this is a breaking change for current installs.

Code

-mkdir -p ~/.mcporter && echo '{"api_key": "paste_your_BANKOFAI_API_KEY_here", "base_url": "https://api.bankofai.io/v1/"}' > ~/.mcporter/bankofai-config.json
+mkdir -p ~/.mcporter && echo '{"api_key": "paste_your_BANKOFAI_API_KEY_here", "base_url": "https://chat.bankofai.io/"}' > ~/.mcporter/bankofai-config.json

Recommendation

Add an :::info or :::caution admonition immediately before the code block:

:::info Upgrading from a previous version?
The BANK OF AI endpoint has changed. If you previously installed OpenClaw Extension,
update your existing config file:

```bash
# Update the base_url in your existing config
sed -i 's|https://api.bankofai.io/v1/|https://chat.bankofai.io/|g' ~/.mcporter/bankofai-config.json

:::


---

#### [MJ-02] `curl | bash` Remote Execution Pattern Not Warned About

| Property | Value |
|----------|-------|
| **Severity** | Major |
| **Category** | Security |
| **File** | `docs/Openclaw-extension/QuickStart.md` : Line 26 (EN); `i18n/zh-Hans/.../QuickStart.md` : Line 26 |

**Description**

> The primary installation command pipes a remote shell script directly into `bash` without any verification step:
>
> ```bash
> curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bash
> ```
>
> The `curl | bash` pattern executes arbitrary remote code with the user's full privileges at download time, with no hash check, no signature verification, and no opportunity to inspect the script content. If the GitHub repository or raw CDN were compromised, users would automatically execute malicious code. The PR adds extensive security tips for wallets and keys but does not address this higher-level supply-chain risk, leaving a gap in the documented security posture.

**Code**
```bash
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bash

Recommendation

Add a brief transparency note and, ideally, an alternative "inspect then run" option:

:::tip You can inspect the script before running it
```bash
# Option A — Inspect first, then run:
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh -o install.sh
# Review install.sh with your editor, then:
bash install.sh

# Option B — Direct run (trusts the source implicitly):
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bash

The script is open-source. You can review it anytime at: https://github.com/BofAI/openclaw-extension/blob/main/install.sh
:::


---

### Minor

#### [MN-01] Node.js Minimum Version Downgrade is Unsubstantiated

| Property | Value |
|----------|-------|
| **Severity** | Minor |
| **Category** | Correctness |
| **File** | `docs/Openclaw-extension/QuickStart.md` : Line 8 (EN); `i18n/zh-Hans/.../QuickStart.md` : Line 8 |

**Description**

> The Node.js minimum version is changed from v20 to v18 in docs and FAQ, but the PR provides no explanation. Node.js v18 reached End-of-Life (EOL) in April 2025. If the installer or MCP servers use features available only in v20+ (e.g., native `fetch`, certain crypto APIs, or ESM behaviour differences), this change could silently cause hard-to-debug runtime failures for users on v18. Conversely, if v18 is genuinely sufficient, the old docs were misleading users into installing a newer version than necessary. Either way, there should be a clear rationale.

**Code**
```diff
-2. **Node.js** (must be v20 or above)
+2. **Node.js** (must be v18 or above)

Recommendation

  1. Verify in package.json / install.sh what the actual tested minimum is.
  2. If v18 is intentional, note in the PR description why (e.g., "MCP server uses no v20-only APIs").
  3. Consider recommending the current LTS (v22 at time of writing) while stating the minimum:
    2. **Node.js** (v18 minimum; v22 LTS recommended)

[MN-02] Medium-Risk Skills Installed by Default Without Explanation

Property Value
Severity Minor
Category Security / Documentation
File docs/Openclaw-extension/QuickStart.md : Lines ~335–345 (skill risk table)

Description

The PR documents a security risk assessment table shown during skill installation:

Skill Gen Socket Snyk
recharge-skill Safe 1 alert Med Risk
x402-payment Med Risk 1 alert Med Risk

The guide simply says "Review the report, then confirm to proceed" — but provides no guidance on what "Med Risk" or "1 alert" means, whether users should be concerned, or what the known issues are. A beginner user following this guide will blindly accept medium-risk packages with no understanding of the implications.

Recommendation

Add a brief explanatory note after the risk table:

:::note About the risk ratings
These ratings come from automated supply-chain scanners:
- **Safe / --**: No known issues at scan time.
- **Med Risk**: The scanner found patterns worth knowing about (e.g., network access, eval usage). These are **not** necessarily malicious but mean the package has non-trivial permissions.
- **1 alert**: A specific dependency has a reported issue.

All skills listed here are maintained by BANK OF AI. If you prefer to review them manually before installing, their source code is available at [github.com/BofAI](https://github.com/BofAI).
:::

[MN-03] Example Auto-Generated Password Displayed as Real Output

Property Value
Severity Minor
Category Documentation
File docs/Openclaw-extension/QuickStart.md : Lines ~194–197 (EN)

Description

The docs show a verbatim "example" auto-generated password:

🔑 Your master password: GS%kE^^n3MVu03*i

While framed as an illustrative example, some users (especially beginners, the primary target audience) may not recognise this as a placeholder and could believe this is a default or safe password to reuse. There's no explicit callout that this is a randomly generated example unique to that install.

Recommendation

Replace the specific password with an obvious placeholder:

🔑 Your master password: <randomly-generated, e.g. GS%kE^^n3MVu03*i>
⚠️ This is unique to your install — write it down now!

Suggestions

[S-01] Sync "Clean Install" Destructive-Action Warning Tone

File: docs/Openclaw-extension/QuickStart.md (Level 1 caution block)
Description: The :::caution block for Clean Install mentions that it "permanently deletes all MCP configs, installed skills, API credentials, and wallet data" but doesn't explicitly call out that wallet funds are not deleted — only the access credentials are lost. A beginner might interpret "wallet data" as losing their cryptocurrency, causing unnecessary panic. Consider adding: "Note: your on-chain funds are never stored locally and cannot be deleted by this operation."


[S-02] Configuration File Reference Table — Add File Creation Conditions

File: docs/Openclaw-extension/QuickStart.md (📋 Configuration File Reference)
Description: The new reference table is excellent, but ~/.x402-config.json is listed without any note on when it is created (it appears only when x402-payment skill is installed). Similarly, .openclaw/skills/ only exists if the user chose option 2. Adding a "Created when" column would prevent confusion for users who don't see these files after a standard install.


[S-03] English and Chinese QuickStart Are Near-Identical — Consider Single-Source

File: All 6 changed files
Description: The English and Chinese documents are structurally identical with translated text. They are currently maintained as two fully duplicated files, meaning every future update must be applied twice (and can easily fall out of sync). Consider using Docusaurus i18n's translation workflow with docusaurus write-translations to manage the Chinese as a translation overlay rather than a full copy, reducing the maintenance surface area.


Positive Observations

Area Observation
Security transparency The PR proactively adds a :::caution block explicitly warning users that the BNB Chain private key is stored in plaintext, recommending a dedicated minimal-fund wallet. This is commendably honest.
Step-by-step UI fidelity Reproducing verbatim installer prompts and output in the docs is a best-practice UX approach that dramatically reduces support burden for beginner users.
Security Tips section The new 🔒 Security Tips section at the end of QuickStart is a well-structured addition that consolidates actionable security advice in one place.
Configuration File Reference The new table listing every config file and its contents is genuinely useful for power users and troubleshooters.
EN/ZH parity All changes are applied consistently to both the English and Chinese documentation, maintaining language parity.
Clean Install confirmation guard The documented CLEAN confirmation requirement for destructive mode is good defensive UX design and the docs represent it accurately.
AgentWallet architecture clarity Elevating AgentWallet to a first-class "Weapon 1" in the Intro correctly communicates its security-critical role versus the prior treatment as a background detail.

Checklist Results

Category Items Checked Pass Fail N/A Notes
Correctness 4 3 1 4 Node.js version downgrade needs substantiation
Security 5 3 2 3 curl|bash not warned; medium-risk skills not explained
Performance 7 0 0 7 Not applicable (docs only)
Code Quality 8 6 0 2 Good structure, clear writing
Testing 6 0 0 6 Not applicable (docs only)
Documentation 6 5 1 0 API URL breaking change lacks migration notice
Compatibility 5 3 1 1 Existing bankofai-config.json will break without migration note
Observability 4 0 0 4 Not applicable (docs only)

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 changed files in this PR are Markdown documentation; no application source code was modified.


Report generated by Code Review Skill v1.0.0
Date: 2026-03-25

@github-actions
Copy link
Copy Markdown

Code Audit Report — PR: update-mcp-servermain

Date: 2026-03-31
Reviewer: Automated Code Review (Claude)
Repository: BofAI / openclaw-extension docs


1. PR Overview

Property Value
Source Branch remotes/origin/update-mcp-server
Target Branch remotes/origin/main
Commits in PR 20 (includes merged PRs: #38, #39 from ai-bankofai-patch-1)
Files Changed 8
Lines Added ~837
Lines Removed ~103

Commit Summary (notable commits)

Hash Message
b85f8e1 update setup
781a1f4 Merge pull request #39 from BofAI/ai-bankofai-patch-1
bfb5e6a add windows setup
1594cb7 update version
e03b329 Update API.md
930728f add openclaw extension
ecfa0c5 Configure "AI's Personal Vault AgentWallet"
fc7883d update openclaw-extension

Files Changed

File Change Type +Lines -Lines
.github/workflows/docker.yml Modified +2 0
docs/Openclaw-extension/FAQ.md Modified +36 -18
docs/Openclaw-extension/Intro.md Modified +22 -22
docs/Openclaw-extension/QuickStart.md Modified +285 -81
i18n/zh-Hans/docusaurus-plugin-content-docs/current/Openclaw-extension/FAQ.md Modified +36 -18
i18n/zh-Hans/docusaurus-plugin-content-docs/current/Openclaw-extension/Intro.md Modified +24 -22
i18n/zh-Hans/docusaurus-plugin-content-docs/current/Openclaw-extension/QuickStart.md Modified +285 -81
package.json Modified +1 -1

2. Change Summary

2.1 CI/CD: GitHub Actions Workflow Update

.github/workflows/docker.yml
Adds ai-bankofai-patch-1 to the branches filter list under both push and pull_request triggers, enabling the Docker build-and-push pipeline to fire for that feature branch.

2.2 Documentation: Windows Platform Support (English)

docs/Openclaw-extension/QuickStart.md, docs/Openclaw-extension/FAQ.md, docs/Openclaw-extension/Intro.md
Major documentation expansion adding full Windows 10/11 support via PowerShell (install.ps1 / install.bat). Changes include:

  • Docusaurus <Tabs> components splitting Mac/Linux vs Windows code examples throughout QuickStart.
  • New Windows-specific FAQ sections (execution policy errors, garbled terminal output, WSL caveat, admin-privilege Q&A).
  • Security tips section including explicit plaintext-key warning for BNB Chain.
  • Configuration file reference table with per-OS paths.
  • Node.js minimum version requirement downgraded from v20 to v18.
  • Python 3 prerequisite removed (no longer required).
  • base_url for BANK OF AI config changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/.

2.3 Documentation: Windows Platform Support (Chinese i18n)

i18n/zh-Hans/docusaurus-plugin-content-docs/current/Openclaw-extension/
Mirrors all English changes in the Chinese translation (Simplified) with parity content.

2.4 Package Version Bump

package.json
Version incremented from 1.2.41.2.6 (note: skips 1.2.5).


3. Detailed Findings

Critical

No critical findings.


Major

[MJ-01] Plaintext Private Key Warning Is Insufficiently Prominent

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 199-216, 451-453

Description: The documentation explicitly acknowledges that BNB Chain private keys are stored in plaintext in ~/.mcporter/mcporter.json (Windows: %USERPROFILE%\.mcporter\mcporter.json). The caution block appears inside the Level 3 wizard walkthrough, but users who skim or skip Level 3 details may still provide their private key. The risk is especially high for Windows users new to Web3 who may not understand the implications of plaintext key storage. The security tips section at the bottom of QuickStart.md only partially compensates, and the Intro.md actively promotes the AgentWallet as "never sends private keys" without clarifying the BNB Chain exception.

Code:

:::caution BNB Chain Private Key
Unlike TRON (which uses the encrypted AgentWallet), BNB Chain currently stores your private key in **plaintext** in the config file. The file permissions are set to 600 (only you can read it), but we strongly recommend using a **dedicated wallet with minimal funds**.
:::

Recommendation:
Add a persistent, top-level callout in the Prerequisites section of QuickStart.md warning that BNB Chain integration stores a private key in plaintext, so users are fully informed before entering their key. Also ensure Intro.md's AgentWallet description clarifies the BNB Chain exception (e.g., add a footnote: "Note: BNB Chain integration currently stores its key in plaintext; TRON uses full encryption via AgentWallet"). Consider adding a dedicated section in FAQ.md under "Passwords and Security Concerns" referencing this limitation.


[MJ-02] irm | iex Installation Pattern Is a Known Security Anti-Pattern

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 54-55

Description: The Windows one-liner irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex instructs users to pipe a remotely fetched script directly into PowerShell for immediate execution. This pattern provides no opportunity for the user to review the script content before running it. If the script or its hosting location were ever compromised (supply chain attack, repository takeover, DNS hijacking), users would be silently executing malicious code with their user-level privileges. The documentation provides no mention of how to inspect the script first, and the Chinese version mirrors this instruction verbatim.

Code:

irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

Recommendation:
Add an explicit note beneath the one-liner instructing users who want to verify first:

> **Want to inspect the script before running it?** Download it first:
> `Invoke-WebRequest -Uri "https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1" -OutFile install.ps1`
> Review the file in a text editor, then run: `powershell -ExecutionPolicy Bypass -File install.ps1`

Also consider publishing a checksum/hash of each release's install.ps1 so security-conscious users can verify integrity.


[MJ-03] Version Bump Skips a Patch Version (1.2.4 → 1.2.6)

Property Value
Severity Major
Category Quality
File package.json : Line 4

Description: The PR bumps the package version from 1.2.4 directly to 1.2.6, skipping 1.2.5. However, git log shows commit 1594cb7 with message "update version" exists earlier in the branch history — suggesting 1.2.5 may have been applied at an intermediate commit and then overwritten or merged in a way that produces this gap. Inconsistent version numbering breaks the semantic versioning contract and makes changelog auditing ambiguous. The main branch already has 1.2.6, meaning this bump was already merged, but the PR's diff still shows the 1.2.4 → 1.2.6 change.

Code:

-  "version": "1.2.4",
+  "version": "1.2.6",

Recommendation:
Clarify whether 1.2.5 was ever released or tagged. If not, update the changelog/release notes to document that no 1.2.5 release exists and that 1.2.6 covers all changes from 1.2.4. Going forward, adopt a policy of bumping versions in a single commit with a corresponding CHANGELOG entry.


Minor

[MN-01] CI Branch Filter Includes Transient Feature Branch

Property Value
Severity Minor
Category Quality / Correctness
File .github/workflows/docker.yml : Lines 9, 17

Description: The Docker build workflow now triggers on pushes to and PRs targeting ai-bankofai-patch-1. This is a specific feature branch name, not a pattern. Once the ai-bankofai-patch-1 branch is eventually deleted after merging, the entry becomes dead configuration noise. More concerning: if someone creates a branch with the same name in future, they will get automatic Docker builds (including a push to Docker Hub for non-PR events) without explicitly opting in.

Code:

branches:
  - main
  - master
  - update-mcp-server
  - ai-bankofai-patch-1

Recommendation:
Remove ai-bankofai-patch-1 from the branch filter once it is merged. Consider using a pattern like feature/** or relying on workflow_dispatch for feature branch builds, rather than permanently listing individual branch names. Alternatively, document a housekeeping policy requiring branch names to be removed from this list after merge.


[MN-02] Mac-Specific open -e ~/.zshrc Instruction Mixed into Mac/Linux Tab

Property Value
Severity Minor
Category Correctness / Docs
File docs/Openclaw-extension/QuickStart.md : Lines 360-368

Description: The "Linux / macOS" tab for Type A API key configuration instructs users to run open -e ~/.zshrc. The open command is macOS-specific; Linux users do not have it. On Linux, the equivalent would be nano ~/.bashrc or xdg-open ~/.bashrc (or ~/.zshrc if using zsh). A Linux user following this instruction will receive command not found: open. Additionally, on Linux, the default shell profile is typically ~/.bashrc or ~/.profile, not ~/.zshrc.

Code:

1. Type `open -e ~/.zshrc` in the terminal and press Enter.

Recommendation:
Split the tab further (macOS vs Linux) or provide platform-conditional instructions:

- **macOS**: Run `open -e ~/.zshrc`
- **Linux (bash)**: Run `nano ~/.bashrc` or your preferred editor
- **Linux (zsh)**: Run `nano ~/.zshrc`

[MN-03] Incomplete FAQ Anchor Link for AgentWallet

Property Value
Severity Minor
Category Docs
File docs/Openclaw-extension/QuickStart.md : Line 163

Description: The link ./FAQ.md#agentwallet-installation-failed points to an FAQ heading. The FAQ section heading is ### AgentWallet Installation Failed. Docusaurus auto-generates anchor IDs from headings. The expected generated anchor would be #agentwallet-installation-failed which matches. However, the same link was previously ./FAQ.md#agentwallet-ai-vault-installation-failed (matching the old heading "AgentWallet (AI Vault) Installation Failed"). This was updated in this PR. No issue with the English link itself; however, the Chinese i18n QuickStart.md links to ./FAQ.md#agentwallet-安装失败了 which targets the Chinese FAQ heading ### AgentWallet 安装失败了. This should work with Docusaurus, but the cross-language anchor behavior depends on the build configuration and should be verified.

Code:

(🚑 Got stuck with an error? 👉 [Click here to see how to fix AgentWallet installation failures](./FAQ.md#agentwallet-installation-failed))

Recommendation:
Verify the generated anchor in a local Docusaurus build for both English and Chinese pages. If the links break, use explicit anchor IDs with {#custom-id} syntax per the Docusaurus heading ID feature to make cross-references stable regardless of heading text changes.


[MN-04] Node.js Minimum Version Inconsistency — Downgraded Without Explanation

Property Value
Severity Minor
Category Correctness / Docs
File docs/Openclaw-extension/QuickStart.md : Line 12; docs/Openclaw-extension/FAQ.md : Line 13

Description: The PR changes the required Node.js minimum version from v20 (stated in the previous docs) to v18. The package.json engines field already specifies "node": ">=18", so the change aligns docs with the actual engine constraint. However, no explanation is provided for the reduction. Users who previously installed v20 per the old instructions will not be affected, but this raises a documentation trust question: why did earlier docs say v20 when the actual minimum is v18?

Code:

-2. **Node.js** (must be v20 or above)
+2. **Node.js** (must be v18 or above)

Recommendation:
Add a brief inline note explaining why v18 is now the stated minimum (e.g., "v18 is sufficient for all skill packs; v20 was previously overstated"). Alternatively, note the actual engines constraint in FAQ.md to reduce future drift.


[MN-05] base_url Changed Without Changelog or Migration Note

Property Value
Severity Minor
Category Docs / Correctness
File docs/Openclaw-extension/QuickStart.md : Lines 400, 408; i18n/zh-Hans/.../QuickStart.md : equivalent lines

Description: The BANK OF AI base URL in the configuration command changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/. This is a significant API endpoint change. Users who previously installed using the old URL will have an outdated bankofai-config.json. There is no migration note, changelog entry, or automated update mechanism mentioned.

Code:

# Old
mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://api.bankofai.io/v1/"}' > ~/.mcporter/bankofai-config.json
# New
mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://chat.bankofai.io/"}' > ~/.mcporter/bankofai-config.json

Recommendation:
Add a migration note in FAQ.md under a new "Upgrading from older versions" section explaining that existing users must update their bankofai-config.json to use the new base URL. Alternatively, note this in the release changelog. The installer's "Normal install" mode should ideally update this value automatically.


[MN-06] Security Risk Assessment Shown to Users Without Interpretation Guidance

Property Value
Severity Minor
Category Docs / Security
File docs/Openclaw-extension/QuickStart.md : Lines 266-276

Description: The QuickStart documentation shows the security risk assessment output for installed skills and advises users to "review the report, then confirm to proceed." However, two skills — recharge-skill (Snyk: Med Risk, Socket: 1 alert) and x402-payment (Gen: Med Risk, Snyk: Med Risk, Socket: 1 alert) — carry flagged risk assessments. The documentation does not interpret what "Med Risk" means, what the Socket alerts indicate, or under what circumstances a user should decline to install. For novice users (the primary target audience), displaying this table without context may either cause undue alarm or, more likely, be ignored entirely.

Code:

◇  Security Risk Assessments
│                                     Gen        Socket        Snyk
│  recharge-skill                     Safe       1 alert       Med Risk
│  x402-payment                       Med Risk   1 alert       Med Risk

Recommendation:
Add a brief paragraph below the risk assessment table explaining what each tool checks (Gen = generative AI scan, Socket = supply-chain/network analysis, Snyk = known vulnerability database) and what "Med Risk" means in practical terms (e.g., "Medium risk flags are informational; they typically relate to the use of network requests or external APIs, which is expected for DeFi tools. If you're concerned, you can skip any skill here and install it manually later."). This respects user autonomy while reducing confusion.


Suggestions

[S-01] Consider Adding a CHANGELOG File or Release Notes Section

Property Value
Severity Suggestion
Category Docs / Maintainability
File Repository root

Description: The PR makes substantive changes: Windows support introduction, BNB Chain plaintext key warning, base URL change, Node.js minimum version correction, and removal of the Python prerequisite. None of these appear in a CHANGELOG or release notes document. Future maintainers reviewing the git log will need to inspect individual commits or diffs to understand the evolution of requirements.

Recommendation:
Create a CHANGELOG.md at the repository root following Keep a Changelog format, with a [1.2.6] entry documenting these changes.


[S-02] The irm | iex One-Liner Targets refs/heads/main (No Version Pin)

Property Value
Severity Suggestion
Category Security / Maintainability
File docs/Openclaw-extension/QuickStart.md : Line 55

Description: The Windows install one-liner always fetches from refs/heads/main, meaning it will always execute whatever the current main branch version of install.ps1 is. This is acceptable for a "latest" install path but means users re-running the command at different times may get different behavior. There is no versioned URL option documented.

Recommendation:
Consider documenting a versioned install URL alongside the main URL:

irm https://raw.githubusercontent.com/BofAI/openclaw-extension/v1.4.5/install.ps1 | iex

This gives users a stable, reproducible installation path when they want to stay on a known version.


[S-03] Linux Terminal Opening Instruction Is Missing

Property Value
Severity Suggestion
Category Docs
File docs/Openclaw-extension/QuickStart.md : Lines 26-27

Description: The terminal opening instructions say for Mac/Linux: "Press Command + Space (Mac) or search Terminal in the app menu". The instruction to search in the "app menu" is vague — different Linux distributions have different launchers (GNOME Activities, KDE Kickoff, XFCE Whisker Menu). A beginner Linux user may not find this helpful.

Recommendation:
Add Ctrl+Alt+T as a common keyboard shortcut that works on most Ubuntu/Debian/GNOME-based desktops:

- **Mac**: Press `Command + Space`, type `Terminal`, and hit Enter.
- **Linux**: Press `Ctrl+Alt+T` (most distros), or search `Terminal` in your application launcher.

[S-04] Docusaurus Tabs Import Placed Inside Body Text (MDX Best Practice)

Property Value
Severity Suggestion
Category Quality / Docs
File docs/Openclaw-extension/QuickStart.md : Lines 29-31

Description: The import Tabs from '@theme/Tabs'; import TabItem from '@theme/TabItem'; statements are placed inline within the document body after the "Open the Terminal" paragraph rather than at the top of the file. Docusaurus MDX is flexible enough to handle this, but the Docusaurus best practices recommend placing all import statements at the top of the file (before any prose), similar to standard JS/TS import organization. The current placement could cause rendering issues with some MDX parsers or future Docusaurus upgrades.

Recommendation:
Move both import lines to the top of the file, immediately after the frontmatter block (or the first heading if there's no frontmatter):

import Tabs from '@theme/Tabs';
import TabItem from '@theme/TabItem';

# Quick Start
...

4. Positive Observations

  • Comprehensive Windows coverage: The Windows platform addition is thorough — installation via PowerShell one-liner (irm | iex), source-based fallback via install.bat, FAQ for execution policy errors, garbled character output, and WSL caveats are all addressed.
  • Well-structured caution callouts: The use of Docusaurus :::caution admonitions for the clean install confirmation, master password importance, and BNB Chain plaintext key warning demonstrates good user-safety-first documentation design.
  • Security tips section: Adding a dedicated "Security Tips" section in QuickStart.md with actionable guidance (dedicated wallet, testnet first, plaintext key warning) is a good practice for a DeFi product targeting non-expert users.
  • Bilingual parity: English and Chinese (Simplified) documentation are kept in sync — both files receive the same structural changes, which is commendable for an internationalized product.
  • Docusaurus Tabs for multi-platform code: Using <Tabs> to split code examples by OS reduces cognitive load and eliminates the risk of users running wrong-platform commands.
  • Python prerequisite removal: Removing the Python 3 dependency (previously listed as a prerequisite) reflects a real simplification in the installation toolchain and reduces the barrier to entry.
  • Installer output examples: Including literal terminal output blocks for each step of the wizard (Level 1 through Level 4) is excellent UX documentation that reduces user uncertainty about whether the installer is working correctly.
  • CI gate maintained: Docker Hub pushes are still gated on github.event_name != 'pull_request', preventing accidental image publishes from PR runs.

5. Checklist Results

Area Status Notes
Correctness Mostly Pass open -e is macOS-only but placed in "Mac/Linux" tab (MN-02); anchor links should be verified in build (MN-03)
Security Needs Attention BNB Chain plaintext key risk documented but under-emphasised (MJ-01); irm | iex pattern lacks verification guidance (MJ-02)
Performance Pass Documentation only; no runtime performance concerns
Code Quality Mostly Pass Version skip 1.2.4→1.2.6 unexplained (MJ-03); CI branch bloat (MN-01)
Testing N/A Documentation repo; no automated test suite present for docs content
Documentation Mostly Pass BNB Chain base URL migration not communicated (MN-05); risk assessment table lacks interpretation (MN-06)
Compatibility Mostly Pass Node.js minimum corrected to v18 aligning with engines field; Windows platform added properly
Observability Pass Installation completion banners and step-by-step output blocks added; users can self-diagnose

6. Review Verdict

Verdict: Request Changes

Rationale

This PR makes a substantial and valuable addition: full Windows platform support for the OpenClaw Extension installer, with bilingual (English + Chinese) documentation coverage. The documentation quality is generally high — use of Docusaurus components, caution callouts, and step-by-step terminal output examples are all best practices.

However, two Major findings require resolution before merging:

  1. MJ-01 — The BNB Chain plaintext private key risk needs to be elevated to a top-of-document warning, not just a caution inside the wizard walkthrough. Users installing on Windows (the new audience) may be less experienced with key security, making this especially important.
  2. MJ-02 — The irm | iex PowerShell one-liner needs accompanying guidance on how to inspect the script before executing it. This is an industry-standard disclosure for any "pipe to shell" installation pattern.

MJ-03 (version skip) is worth documenting in release notes but is not blocking.

The Minor findings (MN-01 through MN-06) and Suggestions (S-01 through S-04) are non-blocking improvements that would increase the overall quality and maintainability of the documentation.

Once MJ-01 and MJ-02 are addressed, this PR is ready for approval.

@github-actions
Copy link
Copy Markdown

Code Review Report

Project: @x402-tron/docs (OpenClaw Extension Documentation)
PR: mainupdate-mcp-server
Review Date: 2026-03-31
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)


PR Overview

Branch Information

Property Value
From Branch main
To Branch update-mcp-server
Commits 14
Files Changed 8
Lines Added +837
Lines Removed -103

Commit History

Hash Message
d27f93f update openclaw extension
b85f8e1 update setup
781a1f4 Merge pull request #39 from BofAI/ai-bankofai-patch-1
bfb5e6a add windows setup
1594cb7 update version
e03b329 Update API.md
3ad439c Update API.md
5617664 Update API.md
2dbed57 Update API.md
98dbee5 Merge pull request #38 from BofAI/ai-bankofai-patch-1
eaff73d delete Duplicate content
c27d6a7 add deepseek-v3.2
930728f add openclaw extension
ecfa0c5 Configure "AI's Personal Vault AgentWallet"

Review Summary

Verdict

Verdict: Request Changes

Findings at a Glance

Critical Major Minor Suggestion
Count 0 2 3 3

Summary

This PR is a substantial documentation update (837 insertions, 103 deletions across 8 files) that adds Windows 10/11 support to the OpenClaw Extension installer guide. The changes include: a full Windows install path with PowerShell commands, Docusaurus <Tabs> components for OS-specific instructions, detailed step-by-step wizard walkthroughs, a new Security Tips section, a Configuration File Reference table, and a new Windows-Specific FAQ section. Chinese (zh-Hans) translations are kept in sync with the English changes. A minor CI change adds a feature branch to Docker build triggers, and package.json is bumped to version 1.2.6.

Overall, the documentation quality is high — the additions are thorough, well-structured, and use good UX patterns (caution blocks, code screenshots, and progressive disclosure). However, there are two Major issues that should be addressed before merge: (1) The Windows install command uses the irm | iex remote code execution pattern without any integrity verification, presenting an unacknowledged security risk to end users; and (2) the API base URL for BANK OF AI silently changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/, which is a functional breaking change for returning users who manually re-run the config snippet. Three minor issues and three suggestions round out the review.


Change Summary

1. Windows Support — Documentation Overhaul (Core Change)

File Change Type Description
docs/Openclaw-extension/QuickStart.md Modified +285 / -46 — Added Windows install path (PowerShell irm|iex, install.bat), Docusaurus Tabs, detailed wizard screenshots, Security Tips section, Config File Reference table
docs/Openclaw-extension/FAQ.md Modified +40 / -14 — Added Windows-specific FAQ section (admin privileges, WSL caveats), new error entries for git, PowerShell execution policy, and garbled terminal output
docs/Openclaw-extension/Intro.md Modified +21 / -20 — Rewrote intro to 3-pillar model (Vault + Toolbox + Skills), added Windows user persona
i18n/zh-Hans/.../QuickStart.md Modified Parallel Chinese translation of all English QuickStart changes
i18n/zh-Hans/.../FAQ.md Modified Parallel Chinese translation of all English FAQ changes
i18n/zh-Hans/.../Intro.md Modified Parallel Chinese translation of all English Intro changes

Purpose: Enables Windows 10/11 users to install OpenClaw Extension natively (v1.4.5+), removing the Mac/Linux-only limitation. Also upgrades the wizard walkthrough from brief descriptions to full step-by-step guidance with actual terminal output examples.

2. CI/CD — Docker Workflow Branch Trigger

File Change Type Description
.github/workflows/docker.yml Modified +2 lines — Adds ai-bankofai-patch-1 to push and pull_request branch triggers

Purpose: Enables the Docker image build to trigger on the ai-bankofai-patch-1 development branch.

3. Package Version Bump

File Change Type Description
package.json Modified Version 1.2.41.2.6; engines.node already set to >=18 (no change)

Purpose: Marks the release of Windows support as a new version.

4. Prerequisite & Dependency Changes

  • Node.js minimum version updated from v20 to v18 in docs (aligning with the existing engines.node: ">=18" in package.json — this is a correction, not a downgrade).
  • Python 3 prerequisite removed from docs entirely. The installer now uses Node.js-based tooling only, eliminating a previously documented dependency.

Detailed Findings


Major

[MJ-01] Windows Install Command Uses Unverified Remote Code Execution (irm | iex) Without Integrity Check

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 48–50; i18n/zh-Hans/.../QuickStart.md : Lines 48–50

Description

The Windows installation command published in the documentation uses the PowerShell irm … | iex pattern:

irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

irm (Invoke-RestMethod) fetches the raw script and iex (Invoke-Expression) executes it directly in the current shell — with the user's full permissions — without any signature verification, hash check, or sandboxing. This is a well-known supply-chain attack surface: if the GitHub raw content endpoint (or a CDN in front of it) is compromised, or if a DNS/BGP hijack occurs, a malicious payload would execute silently on the user's machine.

The equivalent curl | bash pattern on Linux/macOS carries the same risk, but this PR introduces the Windows variant for the first time. The documentation contains no security caveat about this pattern, whereas the BNB Chain private key concern does get a prominent :::caution block. The asymmetry leaves users unaware of the script execution risk.

Code

# docs/Openclaw-extension/QuickStart.md (new Windows tab)
irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

Recommendation

Option A — Add a SHA-256 checksum verification step (preferred):

# Download first, verify hash, then execute
$script = irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1
$hash = (Get-FileHash -InputStream ([IO.MemoryStream]::new([Text.Encoding]::UTF8.GetBytes($script))) -Algorithm SHA256).Hash
if ($hash -ne "EXPECTED_SHA256_HERE") { throw "Script integrity check failed!" }
iex $script

Option B — At minimum, add a visible security note (acceptable short-term):

:::caution Security Note
This command downloads and immediately runs a script from the internet.
If you prefer to review the code first, use the [source install method](#) instead:
`git clone … && install.bat`
:::

The install.bat / source-clone path already documented in this PR is the safer alternative for security-conscious users and should be emphasized.


[MJ-02] Silent API Base URL Change for BANK OF AI Config Snippet Breaks Returning Users

Property Value
Severity Major
Category Correctness
File docs/Openclaw-extension/QuickStart.md : Lines 399–400

Description

The BANK OF AI configuration one-liner changed its base_url value with no mention of the change in the surrounding text, changelog, or upgrade notice:

- "base_url": "https://api.bankofai.io/v1/"
+ "base_url": "https://chat.bankofai.io/"

This is a functional, breaking change for returning users who already have ~/.mcporter/bankofai-config.json on disk with the old URL. When they follow the "How to Add API Keys" section to reconfigure, the new command will write the updated URL — but the text gives no indication that the URL changed or that existing users need to re-run the command. A user who skips this step (because they think they already configured it) will continue sending requests to the old endpoint.

Additionally, the old URL (https://api.bankofai.io/v1/) included an API version path segment (/v1/), while the new URL (https://chat.bankofai.io/) does not. Downstream code that constructs API paths relative to this base URL may produce different results.

Code

# Old (main branch)
mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://api.bankofai.io/v1/"}' > ...

# New (update-mcp-server)
mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://chat.bankofai.io/"}' > ...

Recommendation

  1. Add an explicit migration notice in the affected section:
:::note Upgrading from a previous version?
The BANK OF AI service URL has changed. **Re-run the command below** to update your local config file, even if you configured it before.
:::
  1. Consider whether https://api.bankofai.io/v1/ needs a redirect in place so existing configs don't break hard.
  2. Document the URL change in a changelog or release notes entry.

Minor

[MN-01] Feature Branch ai-bankofai-patch-1 Permanently Added to CI Build Triggers

Property Value
Severity Minor
Category Code Quality
File .github/workflows/docker.yml : Lines 9, 16

Description

The ai-bankofai-patch-1 branch is added to both the push:branches and pull_request:branches triggers in the Docker build workflow. This is a development/patch branch (named after a specific AI contributor's patch), not a permanent long-lived branch. Hardcoding transient feature branches into CI trigger lists is problematic: it accumulates dead entries, triggers unnecessary Docker image builds for every commit on an ephemeral branch, and consumes CI minutes after the branch's PR is merged and the branch deleted.

The commit history shows this branch's PR (#39) was already merged (781a1f4 Merge pull request #39 from BofAI/ai-bankofai-patch-1), suggesting this CI entry may already be stale.

Code

# .github/workflows/docker.yml
on:
  push:
    branches:
      - main
      - master
      - update-mcp-server
+     - ai-bankofai-patch-1   # ← specific patch branch
  pull_request:
    branches:
      - main
      - master
      - update-mcp-server
+     - ai-bankofai-patch-1   # ← same patch branch

Recommendation

Remove ai-bankofai-patch-1 from both trigger lists. If broader branch coverage is needed, consider a glob pattern that matches release/maintenance branches without hardcoding development branches:

branches:
  - main
  - master
  - 'release/**'

[MN-02] Version Skips Patch Release 1.2.5 (Jumped from 1.2.4 to 1.2.6)

Property Value
Severity Minor
Category Code Quality
File package.json : Line 3

Description

The version number jumps from 1.2.4 directly to 1.2.6, skipping 1.2.5. While this may be intentional (e.g., 1.2.5 was published as an interim release outside this PR), there is no comment or changelog entry to indicate this. In a public documentation package, unexplained version gaps create confusion for consumers and make it harder to correlate releases to changes.

Code

- "version": "1.2.4",
+ "version": "1.2.6",

Recommendation

If 1.2.5 was intentionally skipped, add a brief comment or CHANGELOG entry explaining why. If it was accidental, correct to 1.2.5. Ensure the version bump aligns with your published release history on the registry.


[MN-03] Intro.md Rewrite Breaks Existing Paragraph Structure for "Can't Wait?" Section

Property Value
Severity Minor
Category Documentation
File docs/Openclaw-extension/Intro.md : Lines 56–60

Description

The rewritten Intro.md changes the closing CTA ("Can't Wait?") section. The original text was a short, friendly nudge: "Don't hesitate — the entire installation process is as simple as microwaving a meal, taking just a few minutes." The new text reads: "Forget those complicated configuration tutorials. The entire process is guided by a fully automated smart wizard — just copy one command, follow the on-screen prompts and press Enter a few times, and it will take care of everything for you."

The replacement is reasonable in content, but the phrase "Forget those complicated configuration tutorials" is somewhat dismissive of existing documentation the user may have already read elsewhere on the same docs site, and could read as contradicting other sections. More importantly, the Windows-specific 3-step prerequisites (Windows 10 build 1511+, PowerShell 5.1+, Node.js v18) introduced in QuickStart.md mean the "one command does everything" claim is no longer fully accurate for Windows users, creating a potential expectation mismatch.

Recommendation

Soften the claim slightly to align with the Windows prerequisites:

The entire process is guided by a fully automated smart wizard.
For most users: copy one command, follow the on-screen prompts, and you're done.
👉 **[Go to Quick Start and upgrade your AI!](./QuickStart.md)**

Suggestions

[S-01] Security Risk Assessment Table — Add Context for Flagged Skills

File: docs/Openclaw-extension/QuickStart.md (Level 4 skill picker section)

Description: The documentation now includes a raw security risk assessment table reproduced from the installer:

│  recharge-skill    Gen: Safe    Socket: 1 alert    Snyk: Med Risk
│  x402-payment      Gen: Med Risk  Socket: 1 alert  Snyk: Med Risk

Displaying scanner results without any explanation leaves users unable to make an informed decision. "Med Risk" from Snyk and "1 alert" from Socket.dev mean very different things in practice, and a non-technical user will either panic or dismiss both.

Suggestion: Add a brief explanatory note below the table, e.g.:

"These ratings come from automated open-source scanners. 'Med Risk' typically refers to transitive dependencies with known CVEs that don't affect this use case. If you're concerned, you can deselect a skill and add it later after reviewing its source code."


[S-02] Document the --save-runtime-secrets Flag in AgentWallet Setup

File: docs/Openclaw-extension/QuickStart.md : Line (AgentWallet setup section)

Description: The walkthrough shows the installer running:

Launching: agent-wallet start --save-runtime-secrets

The --save-runtime-secrets flag is displayed to users but never explained. The name sounds alarming ("saving secrets") to a security-conscious reader. A brief inline explanation would improve trust.

Suggestion: Add a tooltip or inline note:

--save-runtime-secrets tells AgentWallet to persist your encrypted vault keys to disk so you don't have to re-enter your master password on every restart. Keys remain encrypted at rest.


[S-03] bankofai-config.json Write Command Could Clobber Existing Config on Windows

File: docs/Openclaw-extension/QuickStart.md : Lines 404–409

Description: The Windows PowerShell config snippet uses Out-File which overwrites the target file:

'{"api_key": "...", "base_url": "https://chat.bankofai.io/"}' | Out-File -Encoding utf8 "$env:USERPROFILE\.mcporter\bankofai-config.json"

If a user already has a bankofai-config.json with additional keys (e.g., from a previous install), this command silently destroys them. The Linux equivalent (echo '…' > file) has the same issue, but the Windows path is new and an opportunity to improve.

Suggestion: Use a merge-aware approach or warn about the overwrite:

# Safe: merges with existing config if present
$cfg = if (Test-Path "$env:USERPROFILE\.mcporter\bankofai-config.json") {
    Get-Content "$env:USERPROFILE\.mcporter\bankofai-config.json" | ConvertFrom-Json
} else { [PSCustomObject]@{} }
$cfg | Add-Member -Force api_key "paste_your_BANKOFAI_API_KEY_here"
$cfg | Add-Member -Force base_url "https://chat.bankofai.io/"
$cfg | ConvertTo-Json | Out-File -Encoding utf8 "$env:USERPROFILE\.mcporter\bankofai-config.json"

Or at minimum add a warning callout: "This command overwrites any existing bankofai-config.json. Back it up first if you've customised it."


Positive Observations

Area Observation
Disclosure The BNB Chain plaintext private key risk is called out prominently with a :::caution block — exactly where users need to see it before entering sensitive data.
UX Design Docusaurus <Tabs> component is used correctly throughout, giving clean OS-specific instruction separation without duplicating shared context.
Security Tips section The new dedicated "🔒 Security Tips" section at the end of QuickStart.md consolidates best practices (dedicated wallet, minimal funds, testnet-first) in one scannable place — excellent addition.
Anchor links fixed The old broken anchor #agentwallet-ai-vault-installation-failed is corrected to #agentwallet-installation-failed to match the updated heading.
i18n parity All three English docs (QuickStart.md, FAQ.md, Intro.md) have fully-updated Chinese translations, maintaining locale parity.
Python 3 removal Removing the Python 3 prerequisite correctly reflects the new Node.js-only toolchain, reducing installation friction.
Node.js version alignment Correcting the Node.js requirement from v20 to v18 aligns docs with the engines.node: ">=18" constraint already in package.json, eliminating a misleading inconsistency.
WSL caveat The FAQ explicitly warns against using WSL for native Windows OpenClaw installations, preventing a subtle misconfiguration that would be hard to debug.
Clean install confirmation The CLEAN typed-confirmation guard for destructive reinstalls is well-documented with the actual expected terminal output.

Checklist Results

Category Items Checked Pass Fail N/A Notes
Correctness 5 4 1 3 API URL change (MJ-02) may break returning users
Security 6 4 1 1 irm|iex without integrity check (MJ-01); BNB key plaintext documented but architectural concern remains
Performance 7 0 0 7 No application code changed
Code Quality 6 4 2 0 Stale CI branch (MN-01); version skip (MN-02)
Testing 5 0 0 5 Documentation-only PR
Documentation 6 5 1 0 Missing migration notice for base URL change (MJ-02); security risk table unexplained (S-01)
Compatibility 4 3 0 1 Node.js v18 alignment correct; Windows prerequisites explicit
Observability 3 0 0 3 No application code changed

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 of the referenced installer scripts (install.ps1, install.sh, install.bat), integration with live services, and deployment impact are not covered by this review.


Report generated by Code Review Skill v1.0.0
Date: 2026-03-31

@github-actions
Copy link
Copy Markdown

Code Review Report

Project: x402-tron / BofAI Docs
PR: mainupdate-mcp-server
Review Date: 2026-03-31
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)


PR Overview

Branch Information

Property Value
From Branch remotes/origin/main
To Branch remotes/origin/update-mcp-server
Commits 16
Files Changed 8
Lines Added +995
Lines Removed -103

Commit History

Hash Message
bfba297 update openclaw-extension
d27f93f update openclaw extension
b85f8e1 update setup
781a1f4 Merge pull request #39 from BofAI/ai-bankofai-patch-1
bfb5e6a add windows setup
1594cb7 update version
e03b329 Update API.md
3ad439c Update API.md
5617664 Update API.md
2dbed57 Update API.md
98dbee5 Merge pull request #38 from BofAI/ai-bankofai-patch-1
eaff73d delete Duplicate content
3829c99 Merge branch 'main' into ai-bankofai-patch-1
c27d6a7 add deepseek-v3.2
c2c3309 Update deepseek-v3.2.md
f24ae66 Create deepseek-v3.2.md

Review Summary

Verdict

Verdict: ⚠️ Request Changes

Findings at a Glance

Critical Major Minor Suggestion
Count 0 2 3 2

Summary

This PR is primarily a documentation-only update (plus one CI configuration change and a version bump) focused on adding comprehensive Windows platform support to the OpenClaw Extension installation guide. The changes include expanded QuickStart instructions with OS-specific tabs (Mac/Linux vs Windows), new Windows-specific FAQ entries, and a reworked Intro page. Both English (docs/) and Chinese (i18n/zh-Hans/) documentation are updated in parallel.

The documentation quality improvements are substantial and well-executed: real CLI output is shown at every step, there are clear security callouts for sensitive operations, and a new "Security Tips" section and "Configuration File Reference" table have been added. The i18n translations appear complete and consistent.

However, two Major issues require attention before merge. The PR downgrades the minimum recommended Node.js version from v20 to v18 — a runtime that reached end-of-life on April 30, 2025 (nearly 12 months ago), meaning users who follow these instructions would be running a software with no security patches. Additionally, the new Windows install command follows the irm URL | iex pattern (pipe remote script directly into PowerShell execution) without any integrity verification, which is a recognized supply-chain risk vector. Three minor issues are also noted, including an undocumented API base URL change and a CI branch-trigger expansion.


Change Summary

1. CI/CD Pipeline — Docker Workflow Expansion

File Change Type Description
.github/workflows/docker.yml Modified Added ai-bankofai-patch-1 to both push and pull_request branch triggers

Purpose: Allows the ai-bankofai-patch-1 feature branch to trigger Docker image builds, presumably for testing the changes being merged into this PR.


2. Package Version Bump

File Change Type Description
package.json Modified Version incremented from 1.2.4 to 1.2.6

Purpose: Marks the documentation release that ships Windows support.


3. OpenClaw Extension — English Documentation Overhaul

File Change Type Description
docs/Openclaw-extension/Intro.md Modified Full rewrite: added emoji headers, expanded 3-weapon framework (added AgentWallet as weapon #1), added Windows user persona
docs/Openclaw-extension/QuickStart.md Modified Major expansion: OS-specific install tabs, detailed level-by-level CLI output walkthroughs, Security Tips section, Configuration File Reference table, Windows environment variable instructions
docs/Openclaw-extension/FAQ.md Modified Replaced Python prerequisite with Git; added Windows-specific FAQ section (script execution policy, garbled characters, WSL, admin privileges); expanded path references for Windows

Purpose: Adds first-class Windows 10/11 support to the install documentation, replacing the previously Mac/Linux-only instructions.


4. OpenClaw Extension — Chinese (zh-Hans) Documentation Overhaul

File Change Type Description
i18n/zh-Hans/docusaurus-plugin-content-docs/current/Openclaw-extension/FAQ.md Modified Chinese translation of FAQ changes (mirrors English changes)
i18n/zh-Hans/docusaurus-plugin-content-docs/current/Openclaw-extension/Intro.md Modified Chinese translation of Intro changes (mirrors English changes)
i18n/zh-Hans/docusaurus-plugin-content-docs/current/Openclaw-extension/QuickStart.md Modified Chinese translation of QuickStart changes (mirrors English changes)

Purpose: Keeps the Chinese-language documentation in sync with the English documentation.


Detailed Findings


Major

[MJ-01] Recommended Node.js Version Downgraded to End-of-Life v18

Property Value
Severity Major
Category Security / Documentation
File docs/Openclaw-extension/QuickStart.md : Line 11 · docs/Openclaw-extension/FAQ.md : Line 11 · i18n/zh-Hans/.../QuickStart.md · i18n/zh-Hans/.../FAQ.md

Description

The PR changes the minimum recommended Node.js version from v20 to v18 across four files. Node.js 18 reached its official End-of-Life (EOL) on April 30, 2025 — nearly 12 months before this PR was opened. After EOL, no security patches are issued, meaning users directed to install v18 will be running software with known unpatched vulnerabilities.

Additionally, Node.js 20 reaches EOL in April 2026 (this month). The current actively-maintained LTS release is Node.js 22 (LTS until April 2027).

This regression could expose users — particularly beginners who follow the guide literally — to a security risk from the moment they set up their environment.

Code

# docs/Openclaw-extension/QuickStart.md, line 11
-2. **Node.js** (must be v20 or above): The runtime environment for skill packs.
+2. **Node.js** (must be v18 or above): The runtime environment for skill packs and configuration.

# docs/Openclaw-extension/FAQ.md, line 11
-**How to fix**: Go to the [Node.js official website](https://nodejs.org/) and download the latest stable version (LTS, v20 or higher recommended).
+**How to fix**: Go to the [Node.js official website](https://nodejs.org/) and download the latest stable version (LTS, v18 or higher recommended).

Recommendation

Update the minimum recommended version to v22 (current LTS, supported until April 2027) or at a minimum retain v20 with a note that v22 is now preferred. Update all four affected files:

-2. **Node.js** (must be v18 or above): ...
+2. **Node.js** (must be v22 or above): The runtime environment for skill packs and configuration. *(Node.js 18 and 20 are end-of-life — older versions will cause errors and are no longer receiving security fixes.)*
-download the latest stable version (LTS, v18 or higher recommended).
+download the latest stable version (LTS, v22 or higher recommended).

[MJ-02] Windows Install Command Uses Unverified Remote Script Execution (irm | iex)

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 56–60 · i18n/zh-Hans/.../QuickStart.md (equivalent block)

Description

The new Windows installation command instructs users to pipe a remotely fetched PowerShell script directly into Invoke-Expression without any integrity check:

irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

This is a well-known supply-chain attack vector. If the GitHub account, repository, or CDN serving raw.githubusercontent.com is compromised — or if a DNS/MITM attack redirects the request — arbitrary code will execute with the user's privileges. The analogous Mac/Linux command (curl | bash) carries the same risk, but PowerShell is commonly subject to stricter enterprise security controls and users may not be aware that iex is equivalent to eval.

The documentation provides no guidance on inspecting the script before running it, nor does it publish a SHA-256 checksum or code-signing signature for install.ps1.

Code

# docs/Openclaw-extension/QuickStart.md
irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

Recommendation

  1. Publish a checksum for each release of install.ps1 and document the verification step:
    # Download first, verify, then run
    Invoke-WebRequest -Uri "https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1" -OutFile install.ps1
    # Verify hash (publish expected hash alongside the docs)
    (Get-FileHash install.ps1 -Algorithm SHA256).Hash
    # If hash matches, run:
    powershell -ExecutionPolicy Bypass -File install.ps1
  2. Alternatively, pin to a specific commit SHA in the URL rather than refs/heads/main to prevent a branch-tip substitution attack:
    irm https://raw.githubusercontent.com/BofAI/openclaw-extension/<COMMIT_SHA>/install.ps1 | iex
  3. Add a brief note advising security-conscious users to inspect the script first at the GitHub repository URL before running.

Minor

[MN-01] BNB Chain API Base URL Changed Without Migration Guide for Existing Users

Property Value
Severity Minor
Category Documentation / Compatibility
File docs/Openclaw-extension/QuickStart.md : Lines 469–472

Description

The bankofai-config.json API base URL changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/. This is a meaningful API endpoint change. Existing users who previously installed and configured the extension will have the old URL in ~/.mcporter/bankofai-config.json (Mac/Linux) or %USERPROFILE%\.mcporter\bankofai-config.json (Windows). The docs do not mention this change or provide a migration step.

Code

-mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://api.bankofai.io/v1/"}' > ~/.mcporter/bankofai-config.json
+mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://chat.bankofai.io/"}' > ~/.mcporter/bankofai-config.json

Recommendation

Add a callout to the upgrade path and/or the FAQ noting the base URL change:

:::note Upgrading from a previous install?
The BANK OF AI base URL has changed to `https://chat.bankofai.io/`. If you installed previously, update your config file:

**Mac/Linux:** Edit `~/.mcporter/bankofai-config.json` and change `base_url` to `"https://chat.bankofai.io/"`
**Windows:** Edit `%USERPROFILE%\.mcporter\bankofai-config.json` the same way, or re-run the config command in the "How to Add API Keys" section.
:::

[MN-02] ai-bankofai-patch-1 Feature Branch Added to Production Docker Build Triggers

Property Value
Severity Minor
Category CI/CD / Security
File .github/workflows/docker.yml : Lines 9, 17

Description

The ai-bankofai-patch-1 branch has been added to both push and pull_request build triggers. Any push to this branch will trigger a Docker image build and — if github.event_name != 'pull_request' — will push an image tagged test to Docker Hub. This expands the set of branches that can write to the shared container registry.

The workflow_dispatch trigger is already commented as "Intentionally unrestricted," but adding a specific non-main feature branch to persistent triggers means the branch will continue triggering builds even after the original PR is merged, until the workflow is updated again.

Code

 on:
   push:
     branches:
       - main
       - master
       - update-mcp-server
+      - ai-bankofai-patch-1   # ← will keep triggering after merge

Recommendation

Remove ai-bankofai-patch-1 from the workflow triggers once the PR it supports (PR #39) is merged. If the intent is to keep a staging branch active long-term, document the policy explicitly:

# Consider restricting to permanent branches only:
branches:
  - main
  - master
# Use workflow_dispatch for ad-hoc builds from feature branches

[MN-03] Version Skips 1.2.5 — Non-Sequential Increment

Property Value
Severity Minor
Category Documentation / Code Quality
File package.json : Line 3

Description

The version jumps from 1.2.4 directly to 1.2.6, skipping 1.2.5. While not a functional bug, this creates a gap in the version history that could cause confusion when troubleshooting user-reported issues or when reviewing changelogs.

Code

-  "version": "1.2.4",
+  "version": "1.2.6",

Recommendation

Verify whether 1.2.5 was intentionally reserved or published elsewhere. If not, use 1.2.5 to keep the history sequential. If 1.2.5 was skipped intentionally (e.g., a pre-release), add a comment or CHANGELOG entry explaining the gap.


Suggestions

[S-01] Security Risk Assessment Table in QuickStart Does Not Explain Actionable Next Steps

File: docs/Openclaw-extension/QuickStart.md (Level 4 skill picker section)

Description: The installer shows users a table with Gen / Socket / Snyk risk assessments for each skill (e.g., recharge-skill shows "Med Risk" from Snyk and "1 alert" from Socket). The documentation reproduces this table but does not guide users on what to do if a skill shows Medium Risk — should they skip it? Is it safe to proceed? Without guidance, users either ignore the warning or become unnecessarily alarmed.

Suggestion: Add a short paragraph after the table explaining that Medium Risk typically reflects dependency audit findings (not executable malware), that skills are open-source and can be inspected, and that users with security concerns can review the Socket/Snyk reports at the linked repositories before installing.


[S-02] Mac/Linux curl | bash Install Pattern Also Lacks Integrity Verification

File: docs/Openclaw-extension/QuickStart.md (Linux / macOS tab)

Description: The Linux/macOS install command (curl -fsSL URL | bash) carries the same unverified remote execution risk noted in MJ-02. This existed before this PR but is worth noting for completeness since the PR now draws more attention to installation security via the new "Security Tips" section.

Suggestion: Apply the same checksum/pin recommendations from MJ-02 to the Mac/Linux install command for consistency.


Positive Observations

Area Observation
Documentation depth The QuickStart guide now includes actual CLI output at every step (the installer "game" with Levels 1–4). This dramatically reduces user confusion and support requests.
Windows parity Windows support is comprehensively covered — PowerShell, install.bat, environment variables, ACL permissions — with no gaps between the Mac/Linux and Windows experiences.
Security transparency The documentation explicitly warns users that BNB Chain private keys are stored in plaintext and recommends using a dedicated wallet with minimal funds. This is the right approach.
Double confirmation for destructive ops The Clean install flow requires users to type CLEAN to confirm deletion of all wallet data — a strong safeguard against accidental data loss.
Security Tips section The new dedicated "Security Tips" section (docs/Openclaw-extension/QuickStart.md) consolidates all security guidance in one scannable location.
Config file reference table The tabbed reference table listing all sensitive config file paths (with OS-specific variants) is excellent for troubleshooting and auditing.
i18n consistency All three Chinese translation files (FAQ.md, Intro.md, QuickStart.md) are updated to match the English changes, maintaining parity across languages.
Prerequisite simplification Removing Python 3 as a prerequisite reduces the installation burden; replacing it with Git (already needed for skill packs) simplifies the requirements list.

Checklist Results

Category Items Checked Pass Fail N/A Notes
Correctness 4 4 0 4 Doc-only; logic correctness N/A for most checks
Security 6 3 2 1 EOL Node.js rec (MJ-01); irm|iex unverified (MJ-02)
Performance 7 0 0 7 No code logic changes
Code Quality 6 4 1 1 Version skip (MN-03)
Testing 5 0 0 5 Documentation PR; no test coverage applicable
Documentation 6 5 1 0 Base URL migration guide missing (MN-01)
Compatibility 5 4 1 0 Node.js v18 EOL affects compatibility guidance
Observability 4 0 0 4 No code logic changes

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.


Report generated by Code Review Skill v1.0.0
Date: 2026-03-31

@github-actions
Copy link
Copy Markdown

Code Review Report

Project: BofAI / openclaw-extension Docs
PR: mainupdate-mcp-server
Review Date: 2026-03-31
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)


PR Overview

Branch Information

Property Value
From Branch main
To Branch update-mcp-server
Commits 23
Files Changed 22
Lines Added +1,235
Lines Removed -151

Commit History

Hash Message
7f26fd6 update agent-wallet and skills
bfba297 update openclaw-extension
d27f93f update openclaw extension
b85f8e1 update setup
781a1f4 Merge pull request #39 from BofAI/ai-bankofai-patch-1
bfb5e6a add windows setup
1594cb7 update version
e03b329 Update API.md
3ad439c Update API.md
5617664 Update API.md
2dbed57 Update API.md
98dbee5 Merge pull request #38 from BofAI/ai-bankofai-patch-1
eaff73d delete Duplicate content
3829c99 Merge branch 'main' into ai-bankofai-patch-1
c27d6a7 add deepseek-v3.2
c2c3309 Update deepseek-v3.2.md
... (additional merge/patch commits)

Review Summary

Verdict

Verdict: Request Changes

Findings at a Glance

Critical Major Minor Suggestion
Count 0 4 4 3

Summary

This PR is a documentation-only update that introduces two new skill entries (trc20-toolkit-skill and multisig-permissions), full Windows platform support across the OpenClaw Extension guides, Agent Wallet default ID rename (defaultdefault_secure), and Node.js minimum version reduction (v20 → v18). The Chinese (zh-Hans) localization files mirror all English changes faithfully. A version bump from 1.2.4 to 1.2.6 in package.json is also included.

The documentation quality overall is high — the new Windows wizard walkthrough is detailed and well-structured. However, several issues need attention before merge: a security-sensitive Windows installation command (irm | iex) is surfaced without any hash/integrity verification guidance; the BNB Chain plaintext private-key storage risk, while disclosed, needs stronger actionable mitigations; an unrelated feature branch (ai-bankofai-patch-1) is permanently added to the Docker CI trigger; and two minor inconsistencies in the security scan tables need correction for user trust.


Change Summary

1. New Skills Documentation: trc20-toolkit-skill & multisig-permissions

File Change Type Description
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md Modified Added full sections for trc20-toolkit-skill and multisig-permissions
docs/McpServer-Skills/SKILLS/Intro.md Modified Skill count 5→7; added feature blurbs for the two new skills
docs/McpServer-Skills/SKILLS/Faq.md Modified Updated skill directory listing to include new skill names
docs/McpServer-Skills/SKILLS/QuickStart.md Modified Skill count 6→7; added TRC20 Token Toolkit to installer output
(zh-Hans mirrors) Modified All Chinese equivalents updated to match

Purpose: Document two new skills shipped in v1.2.6. Adds complete usage examples, read-only vs. write-operation distinctions, and real-world scenario prompts for each skill.


2. Windows Platform Support

File Change Type Description
docs/Openclaw-extension/QuickStart.md Modified (+330 lines) Full Windows install wizard walkthrough, tab-based Mac/Linux vs. Windows instructions, irm|iex PowerShell command, config file reference, security tips
docs/Openclaw-extension/FAQ.md Modified Windows-specific error entries; node requirement v20→v18; Python3→Git prerequisite; Windows uninstall paths
docs/Openclaw-extension/Intro.md Modified Major rewrite: restructured into three-weapon (Vault/Toolbox/Skills) model; Windows user persona added
(zh-Hans mirrors) Modified All Chinese equivalents updated

Purpose: Extend platform support documentation to Windows 10/11 (PowerShell installer), matching the v1.4.5 feature release.


3. Agent Wallet Default ID Rename & Clarifications

File Change Type Description
docs/Agent-Wallet/QuickStart.md Modified Default wallet ID defaultdefault_secure; updated CLI output screenshots
docs/Agent-Wallet/Developer/CLI-Reference.md Modified Added password retry behavior info callout (3 total attempts); default wallet ID note
docs/Agent-Wallet/Developer/SDK-Guide.md Modified Clarified InsufficientBalanceError is not SDK-thrown; added tip callout
(zh-Hans mirrors) Modified All Chinese equivalents updated

Purpose: Reflect the actual default wallet ID change and clarify SDK error contract.


4. CI/CD & Package Changes

File Change Type Description
.github/workflows/docker.yml Modified Added ai-bankofai-patch-1 to push and pull_request branch triggers
package.json Modified Version bump 1.2.41.2.6

Purpose: Version tracking; CI trigger expanded to cover an additional branch.


Detailed Findings


Major

[MJ-01] Windows irm | iex Installation Pattern Lacks Integrity Verification

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 598–601 (EN), i18n/zh-Hans/.../QuickStart.md lines ~1657–1658

Description

The recommended Windows install command irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex pipes a live script directly into Invoke-Expression without any hash verification or signature check. While this pattern is common in the ecosystem, it is a well-known supply-chain attack vector: if the GitHub repository or CDN is compromised, users will silently execute malicious code with their user-level privileges. No guidance is provided to users on how to verify the script's integrity before execution.

Code

irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

Recommendation

Add a note advising users to verify the script hash before running, or provide a pinned commit-SHA URL as the recommended path:

# Pinned to a specific release commit (safer):
irm https://raw.githubusercontent.com/BofAI/openclaw-extension/<COMMIT_SHA>/install.ps1 | iex

# Or: download first, inspect, then run:
Invoke-WebRequest -Uri "https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1" -OutFile install.ps1
# Review the script, then:
powershell -ExecutionPolicy Bypass -File install.ps1

Add a callout:

:::caution Verify before you run
For maximum safety, download the script first, review it, then run it manually.
See [install.ps1 on GitHub](https://github.com/BofAI/openclaw-extension/blob/main/install.ps1).
:::

[MJ-02] BNB Chain Private Key Plaintext Storage Risk Is Under-Mitigated

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines ~811–826

Description

The docs correctly call out that the BNB Chain MCP server stores the user's private key in plaintext in ~/.mcporter/mcporter.json (or %USERPROFILE%\.mcporter\mcporter.json on Windows). The caution block recommends using "a dedicated wallet with minimal funds" but provides no further mitigation. Unlike the TRON side which uses the encrypted AgentWallet, this is a permanent architectural gap. A user who follows the default "select all MCP servers" flow may paste a real private key here without fully understanding the risk. The recommendation to use a "minimal funds" wallet is necessary but not sufficient — the docs do not explain what happens if mcporter.json is compromised (e.g., by another process that reads the user home directory).

Code

⚠ Your PRIVATE_KEY will be stored in plaintext in: ~/.mcporter/mcporter.json
...
:::caution BNB Chain Private Key
Unlike TRON (which uses the encrypted AgentWallet), BNB Chain currently stores
your private key in **plaintext** in the config file. The file permissions are
set to 600 (only you can read it), but we strongly recommend using a **dedicated
wallet with minimal funds**.
:::

Recommendation

Strengthen the caution with:

  1. A clear statement that the file is only as secure as the operating system user account.
  2. Advice to use a hardware-derived key or a new throwaway key.
  3. A link to a future roadmap item for encrypted BNB Chain key storage (if one exists).
:::caution BNB Chain Private Key — High Risk
Unlike TRON (which uses encrypted AgentWallet), BNB Chain stores your private
key **in plaintext** in `mcporter.json`. Anyone with access to your user account
can read this file.

**We strongly recommend:**
- Generate a **brand-new, dedicated key** with near-zero balance (gas fees only).
- Never reuse a key that holds significant assets.
- On Linux/macOS the installer sets `chmod 600`; on Windows it uses `icacls` ACL.
  This only protects against other OS users — it does NOT protect against malware
  running as your own user account.

If you don't have a BNB Chain key, press Enter to skip — you can add it later.
:::

[MJ-03] Inconsistent Security Scan Results Between Two QuickStart Docs

Property Value
Severity Major
Category Documentation
File docs/Openclaw-extension/QuickStart.md : Lines ~882–892 vs. docs/McpServer-Skills/SKILLS/QuickStart.md : Lines ~91–100

Description

The two QuickStart documents present contradictory security scan results for the same skills. In McpServer-Skills/SKILLS/QuickStart.md, recharge-skill shows -- | -- | -- (no issues detected). In Openclaw-extension/QuickStart.md, recharge-skill shows Safe | 1 alert | Med Risk. Similarly, x402-payment shows Med | 1 alert | Med in the Skills QuickStart but Safe | 0 alerts | Med Risk in the Openclaw QuickStart. Users reading both docs will get different impressions of the risk profile of the same skills. This erodes trust and creates confusion.

Code

# In docs/McpServer-Skills/SKILLS/QuickStart.md:
│  recharge-skill                    --     --       --    │
│  x402-payment                      Med    1 alert  Med   │

# In docs/Openclaw-extension/QuickStart.md:
│  recharge-skill                    Safe       1 alert       Med Risk
│  x402-payment                       Safe       0 alerts      Med Risk

Recommendation

Synchronize the two tables so they reflect the same scan results. If the scans were run at different times, re-run them and update both docs from the same run. If intentionally different (e.g., different scanner configurations), add a note explaining the discrepancy.


[MJ-04] ai-bankofai-patch-1 Feature Branch Permanently Added to Docker CI Triggers

Property Value
Severity Major
Category Compatibility / CI/CD
File .github/workflows/docker.yml : Lines 9, 17

Description

The Docker build/test CI workflow is now permanently triggered on every push and pull_request targeting the ai-bankofai-patch-1 branch. This appears to be an incidental branch that was merged via PRs #38 and #39. Permanently embedding a feature branch in the base workflow means: (a) if the branch is deleted and recreated, CI fires on unrelated work; (b) CI resource costs increase for every push to that branch; (c) it sets a precedent of committing transient branch names to the pipeline. The workflow_dispatch comment already notes it is "intentionally unrestricted," which makes this additional trigger unnecessary.

Code

on:
  push:
    branches:
      - main
      - master
      - update-mcp-server
+     - ai-bankofai-patch-1   # <-- feature branch
  pull_request:
    branches:
      - main
      - master
      - update-mcp-server
+     - ai-bankofai-patch-1   # <-- feature branch

Recommendation

Remove the ai-bankofai-patch-1 entries. If CI coverage is needed for active branches, consider a wildcard pattern:

branches:
  - main
  - master
  - update-mcp-server
  - 'release/**'

Or rely on workflow_dispatch for ad-hoc branch builds.


Minor

[MN-01] Example Master Password Shown in Documentation

Property Value
Severity Minor
Category Security / Documentation
File docs/Openclaw-extension/QuickStart.md : Lines ~728–729, ~757–758; i18n/zh-Hans/.../QuickStart.md equivalent lines

Description

The documentation shows a real-looking auto-generated master password (7#KQoc&%m4S7$Dhk) in multiple code block examples. While this is clearly illustrative, users in a hurry might copy this value directly into the password prompt rather than letting the system auto-generate a unique one. Additionally, repeating this exact string twice in the same document reinforces it as if it were a real default.

Recommendation

Replace the example password with an obviously placeholder value:

🔑 Your master password: <auto-generated-password-will-appear-here>

[MN-02] Node.js Minimum Version Requirement Lowered Without Stating Compatibility Rationale

Property Value
Severity Minor
Category Documentation / Compatibility
File docs/Openclaw-extension/QuickStart.md : Line ~554; docs/Openclaw-extension/FAQ.md : Lines ~366, ~420

Description

The Node.js minimum requirement was silently changed from v20 to v18 across all affected files. While this may broaden compatibility for users with older installations, there is no mention of what drove this change or whether it has been tested at v18. Node.js 18 reached End-of-Life on April 30, 2025. Recommending an EOL runtime in new documentation is a security concern and may frustrate users who encounter compatibility issues.

Code

-2. **Node.js** (must be v20 or above)
+2. **Node.js** (must be v18 or above)

Recommendation

Either:

  1. Restore the v20 minimum requirement (still in active LTS), or
  2. Keep v18 but add a note: "Node.js 18 is the minimum. Node.js 20 or 22 (LTS) is recommended as Node.js 18 has reached End-of-Life."

[MN-03] Base URL for BANK OF AI Changed Without Changelog Note

Property Value
Severity Minor
Category Documentation / Compatibility
File docs/Openclaw-extension/QuickStart.md : Lines ~1011–1012

Description

The BANK OF AI config base_url changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/. This is a breaking change for any user who followed previous documentation and manually set the old URL. There is no migration note, no mention that existing configs must be updated, and no explanation for the change.

Code

-mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://api.bankofai.io/v1/"}' > ...
+mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://chat.bankofai.io/"}' > ...

Recommendation

Add a migration note for existing users:

:::note Existing users
If you previously configured `base_url` as `https://api.bankofai.io/v1/`, update it to `https://chat.bankofai.io/` in `~/.mcporter/bankofai-config.json`. The old endpoint has been deprecated.
:::

[MN-04] Section Header Simplification Reduces Discoverability

Property Value
Severity Minor
Category Documentation
File docs/McpServer-Skills/SKILLS/BANKOFAISkill.md : Lines ~121, ~130, ~139, ~220, ~222`

Description

Multiple section headers were simplified by removing their descriptive subtitles. For example:

  • ## sunswap — Swap Tokens, Check Prices, Manage Pools## sunswap
  • ## sunperp-skill — Perpetual Contract Trading## sunperp-skill
  • ## tronscan-skill — On-Chain Data Detective## tronscan-skill
  • ## x402-payment — On-Chain "Pay-First" Auto-Settlement## x402-payment
  • ## recharge-skill — BANK OF AI Account Management## recharge-skill

While this shortens the table of contents, users skimming the page or reading the sidebar navigation now see only opaque identifiers rather than human-readable descriptions, making it harder to find the right skill quickly — especially for newcomers.

Recommendation

Retain the descriptive subtitles or move them to a subheading immediately below the anchor heading:

## sunswap {#sunswap}
### Swap Tokens, Check Prices, Manage Liquidity Pools

Suggestions

[S-01] Add a Version Pin Option for Windows irm Install

File: docs/Openclaw-extension/QuickStart.md
Description: Alongside the rolling main-branch URL, offer a release-tag–pinned URL so users can install a specific version reproducibly and reduce exposure to live HEAD changes.
Suggestion: Add a comment or secondary code block pointing to a tagged release:

# Install specific version (recommended for production/automated setups):
irm https://github.com/BofAI/openclaw-extension/releases/download/v1.4.5/install.ps1 | iex

[S-02] Add a --version Verification Step to Post-Install Validation

File: docs/Openclaw-extension/QuickStart.md
Description: The post-install "test" commands ("Read the recharge-skill...") rely on the AI chat interface to verify success. Adding a CLI command like npx skills list -g (already mentioned in the completion output block) to the main walkthrough text would give users a deterministic, offline way to confirm the installation.
Suggestion: Add a verification step after the Installation Complete block:

**Verify your install in a terminal:**
```bash
npx skills list -g

You should see all 7 skills listed.


---

#### [S-03] Cross-Reference the Security Tips Section from the Caution Callouts

**File:** `docs/Openclaw-extension/QuickStart.md`
**Description:** The new `🔒 Security Tips` section at the bottom of QuickStart.md is well-written but is not linked from the inline caution blocks about the BNB Chain private key or AgentWallet password. Users who dismiss the callouts mid-installation may never see it.
**Suggestion:** Add a `👉 See [Security Tips](#-security-tips) for full guidance.` line inside the BNB Chain caution and the AgentWallet password caution.

---

## Positive Observations

| Area | Observation |
|------|-------------|
| **Documentation depth** | The new Windows QuickStart walkthrough is exceptionally thorough — every installer prompt is shown with exact expected output, reducing user confusion dramatically. |
| **Security transparency** | The BNB Chain private-key plaintext limitation is disclosed prominently with a caution block, demonstrating honest documentation practices. |
| **Bilingual parity** | All English documentation changes are faithfully mirrored in the `zh-Hans` localization files, ensuring neither audience is left behind. |
| **Skill read-only labeling** | Both new skill sections (`trc20-toolkit-skill`, `multisig-permissions`) clearly separate "completely safe — looking only" operations from "requires your confirmation" operations, giving users clear risk awareness. |
| **Clean install flow** | The two-step "type CLEAN to confirm" verification for the destructive clean-install mode is good UX for preventing accidental data loss. |
| **InsufficientBalanceError clarification** | The new tip callout explaining that `InsufficientBalanceError` is caller-provided (not SDK-thrown) is a valuable API contract clarification that prevents developer confusion. |
| **Multi-sig templates** | The disclosure of built-in permission templates (`basic-2of3`, `agent-restricted`, `team-tiered`, `weighted-authority`) helps users understand the capability scope without being overwhelmed. |

---

## Checklist Results

| Category | Items Checked | Pass | Fail | N/A | Notes |
|----------|:---:|:---:|:---:|:---:|-------|
| Correctness | 5 | 4 | 1 | 2 | MN-02: EOL Node.js version lowering |
| Security | 6 | 3 | 3 | 0 | MJ-01: irm\|iex; MJ-02: BNB plaintext key; MN-01: example password |
| Performance | 3 | 3 | 0 | 0 | No performance-sensitive changes |
| Code Quality | 5 | 3 | 2 | 0 | MN-04: header simplification; MJ-03: scan inconsistency |
| Testing | 2 | 2 | 0 | 2 | Documentation-only PR |
| Documentation | 6 | 4 | 2 | 0 | MN-03: no migration note for URL change; MN-04: section header regression |
| Compatibility | 3 | 2 | 1 | 0 | MJ-04: feature branch in CI; MN-02: Node 18 EOL |
| Observability | 2 | 2 | 0 | 2 | 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-31*

@github-actions
Copy link
Copy Markdown

Code Review Audit Report

Date: 2026-03-31
Reviewer: Claude Sonnet 4.6 (Automated Review)
PR: mainupdate-mcp-server
Repository: /app/reports/docs/pr/20260331-130855/code


1. PR Overview

Branch Information

Property Value
Base Branch remotes/origin/main
Head Branch remotes/origin/update-mcp-server
Total Commits 5 (930cbcc, 7f26fd6, bfba297, d27f93f, b85f8e1)
Files Changed 24
Insertions +1,239
Deletions -155

Commit Summary

Commit Message
930cbcc Address Masking
7f26fd6 update agent-wallet and skills
bfba297 update openclaw-extension
d27f93f update openclaw extension
b85f8e1 update setup

Files Changed

File Category
.github/workflows/docker.yml CI/CD
package.json Version Bump
docs/Agent-Wallet/Developer/CLI-Reference.md Documentation
docs/Agent-Wallet/Developer/SDK-Guide.md Documentation
docs/Agent-Wallet/QuickStart.md Documentation
docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md Documentation
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md Documentation
docs/McpServer-Skills/SKILLS/Faq.md Documentation
docs/McpServer-Skills/SKILLS/Intro.md Documentation
docs/McpServer-Skills/SKILLS/QuickStart.md Documentation
docs/Openclaw-extension/FAQ.md Documentation
docs/Openclaw-extension/Intro.md Documentation
docs/Openclaw-extension/QuickStart.md Documentation
i18n/zh-Hans/... (11 mirrored files) i18n (Chinese)

2. Change Summary

A. New Skills Documentation (Two New Skills)

The PR adds complete documentation for two new MCP skills:

  • trc20-toolkit-skill: TRC20 token balance, transfer, and approval management
  • multisig-permissions: TRON account multi-signature and permission management

Both skills are added to the skill summary table, the skill catalog (BANKOFAISkill.md), the skills intro (Intro.md), FAQ skill listing, and the QuickStart install count (6 → 7 skills).

B. Windows Support Documentation

A significant addition: Windows 10/11 is now a first-class supported platform. Changes include:

  • Two-tab (macOS/Linux vs Windows) install instructions in QuickStart.md
  • PowerShell-based installer (install.ps1) and launcher (install.bat) documented
  • Windows-specific FAQ entries added
  • Windows paths (%USERPROFILE%\) documented throughout
  • Security tips extended to cover icacls ACL equivalence to chmod 600

C. Prerequisite Changes

  • Node.js minimum version requirement reduced from v20 to v18
  • Python 3 requirement removed entirely; replaced by a Git requirement for skill downloads
  • PowerShell 5.1+ added as a Windows-specific requirement

D. Wallet Default ID Update

  • Default wallet ID changed from default to default_secure in Agent-Wallet docs and QuickStart

E. API Base URL Change

  • base_url in bankofai-config.json changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/

F. Address Masking

  • Real TRON addresses in documentation examples replaced with truncated placeholders (e.g., TDqSq...xxxxx, TNPee...xxxxx)

G. CI/CD Workflow Update

  • Added branch ai-bankofai-patch-1 to the Docker build triggers (push and pull_request events)

H. Package Version Bump

  • package.json version bumped from 1.2.4 to 1.2.6 (skips 1.2.5)

I. Security Risk Table Discrepancy (QuickStart docs vs SKILLS)

The x402-payment risk rating differs between documents — noted in the Detailed Findings.


3. Detailed Findings


[severity-01] API Base URL Changed Without Migration Guide

Property Value
Severity Major
Category Correctness / Docs
File docs/Openclaw-extension/QuickStart.md : Lines 481, 489; i18n/zh-Hans/.../QuickStart.md

Description: The base_url in the bankofai-config.json example changed from https://api.bankofai.io/v1/ to https://chat.bankofai.io/. Any user who previously set up the config using the old documentation will have a non-functional config without knowing it. The PR does not mention this as a breaking change, does not provide a migration step (e.g., a note saying "if you installed before, update your config"), and does not explain why the URL changed. If the old endpoint was deprecated or redirected, that should be stated.

Code:

# Old (main branch)
"base_url": "https://api.bankofai.io/v1/"

# New (update-mcp-server branch)
"base_url": "https://chat.bankofai.io/"

Recommendation: Add a callout (e.g., :::caution Upgrading from a previous install?) explaining that existing users must update their bankofai-config.json. If the old URL redirects automatically, document that. If it is a hard break, mark it as a breaking change in the PR description and CHANGELOG.


[severity-02] BNB Chain Private Key Plaintext Storage Lacks Sufficient Warnings

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 258–276

Description: The new Windows installation section documents that BNB Chain still stores the private key in plaintext in ~/.mcporter/mcporter.json. While a :::caution block is present, the overall framing encourages users to "enter PRIVATE_KEY (optional)" as a standard step of installation, which normalizes the insecure pattern. There is no warning that this risk fundamentally differs from the AgentWallet-backed TRON setup, and no suggestion of an alternative (e.g., use a hardware wallet, use a dedicated throwaway key). The security tips section at the end does note "minimal funds" but this is buried far from the prompt.

Code:

⚠ Your PRIVATE_KEY will be stored in plaintext in: ~/.mcporter/mcporter.json
? Enter BNB Chain PRIVATE_KEY (optional):

Recommendation:

  1. Move the caution closer to where the user is prompted — directly above the prompt, not only in a sidebar.
  2. Consider adding a risk severity level (e.g., "HIGH RISK: plaintext key") so it stands out.
  3. Recommend generating a dedicated BNB Chain wallet with minimal funds before this step, and link to a guide.

[severity-03] Node.js Version Requirement Downgraded Without Justification

Property Value
Severity Major
Category Correctness / Docs
File docs/Openclaw-extension/QuickStart.md : Line 12; docs/McpServer-Skills/SKILLS/QuickStart.md; multiple i18n files

Description: The Node.js minimum version requirement was changed from v20 to v18. Node.js v18 reached end-of-life on April 30, 2025 — it is no longer receiving security updates. Downgrading the documented minimum version to an EOL release introduces a security risk for users who follow the documentation literally and install v18. There is no explanation in the PR description or commit messages for this downgrade. If the actual runtime does support v18 (e.g., for broader compatibility), this should be called out explicitly with the security caveat.

Code:

# Before
Node.js (must be v20 or above)

# After
Node.js (must be v18 or above)

Recommendation:

  • If v18 support was intentionally added for compatibility, document the rationale and add a note that v18 is EOL and v20+ is strongly recommended.
  • If the actual installer enforces v20+ at runtime, this documentation is actively misleading.
  • Verify and align the documented minimum with the actual runtime check.

[severity-04] ai-bankofai-patch-1 Branch Added to Docker Build Trigger

Property Value
Severity Major
Category Security / Quality
File .github/workflows/docker.yml : Lines 9, 15

Description: The ai-bankofai-patch-1 branch is a feature/patch branch (visible in the remote branch list) that has now been added to both the push trigger and the pull_request trigger for the Docker build and push workflow. This means any push to this branch — including from contributors — triggers a Docker build and pushes to Docker Hub. Feature branches should generally not trigger Docker Hub pushes; this could result in development or untested images being published to the production registry. Additionally, the branch name pattern hardcoding is fragile — as more feature branches are added, the list grows.

Code:

on:
  push:
    branches:
      - main
      - master
      - update-mcp-server
      - ai-bankofai-patch-1   # <-- added in this PR
  pull_request:
    branches:
      - main
      - master
      - update-mcp-server
      - ai-bankofai-patch-1   # <-- added in this PR

Recommendation:

  • Remove ai-bankofai-patch-1 from the Docker push trigger. Feature branch builds should not push to the shared Docker registry.
  • Consider using a glob pattern (e.g., release/*) rather than hardcoding branch names.
  • The push: event should only include long-lived stable branches (main, master).

[severity-05] Security Risk Table Inconsistency Between Documents

Property Value
Severity Minor
Category Docs / Correctness
File docs/McpServer-Skills/SKILLS/QuickStart.md : Lines 91–96 vs docs/Openclaw-extension/QuickStart.md : Lines 327–335

Description: The security risk assessment tables in the two QuickStart documents show different ratings for recharge-skill and x402-payment:

  • docs/McpServer-Skills/SKILLS/QuickStart.md: recharge-skill shows --, x402-payment shows Med / 1 alert / Med
  • docs/Openclaw-extension/QuickStart.md: recharge-skill shows Safe / 1 alert / Med Risk, x402-payment shows Safe / 0 alerts / Med Risk

These are supposed to represent the same security scans. The discrepancy is likely due to documentation written at different times without cross-checking, but it creates confusion — a user following one guide gets a different picture of the same tool's risk profile.

Recommendation: Synchronize the security risk tables across both QuickStart documents so they show the same values. Pick the most current/accurate scan result and apply it everywhere.


[severity-06] Package Version Skips a Patch Number

Property Value
Severity Minor
Category Quality
File package.json : Line 3

Description: The version was bumped from 1.2.4 to 1.2.6, skipping 1.2.5. While not technically incorrect, this is unusual and can cause confusion in changelogs, release notes, and package registries. If 1.2.5 was used for an internal tag, hotfix, or aborted release, that should be documented.

Code:

- "version": "1.2.4",
+ "version": "1.2.6",

Recommendation: Either confirm that 1.2.5 was intentionally skipped (and document why) or use 1.2.5 as the next version. If a 1.2.5 tag exists on another branch or was published privately, note it.


[severity-07] Python 3 Prerequisite Removed Without Explanation

Property Value
Severity Minor
Category Docs / Correctness
File docs/Openclaw-extension/QuickStart.md : Lines 9–14; i18n/zh-Hans/.../QuickStart.md

Description: Python 3 was listed as a prerequisite ("A helper used by the installation wizard to process configuration files") and has been silently removed in this PR without any explanation in the commit messages or PR description. If the installer was refactored away from Python, that is a significant internal change that the docs should briefly acknowledge. If the removal is incorrect (Python is still required at runtime), this omission could cause installation failures for users who follow the updated documentation.

Recommendation: Add a brief note in the commit or PR description explaining that Python is no longer required. Consider adding a sentence in the QuickStart that specifically says "No Python required" to proactively address users who recall the old requirement.


[severity-08] irm | iex PowerShell Pattern Has Security Implications Not Documented

Property Value
Severity Minor
Category Security / Docs
File docs/Openclaw-extension/QuickStart.md : Windows install tab

Description: The Windows installation command irm <url> | iex (Invoke-RestMethod piped to Invoke-Expression) is documented as the recommended install method. This pattern downloads and immediately executes arbitrary remote code, which is a known attack vector if the URL is compromised, the CDN is hijacked, or the user misreads/miscopies the URL. While curl | bash has the same risk on Linux/Mac (and is already documented), the Windows version deserves an equivalent security acknowledgment. Currently the docs frame it identically to the Mac/Linux version without noting this risk.

Recommendation: Add a short note (similar to how the BNB Chain key warning is handled) that users should verify the URL and that this command executes remote code. A hash verification or signature verification step would be ideal, even if just as a "for advanced users" aside.


[severity-09] Truncated Address Examples May Be Confusing to New Users

Property Value
Severity Minor
Category Docs / Quality
File docs/McpServer-Skills/SKILLS/BANKOFAISkill.md; docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md; i18n counterparts

Description: Real example TRON addresses (e.g., TDqSquXBgUCLYvYC4XZgrprLK589dkhSCf, TNPeeaaFB7K9cmo4uQpcU32zGK8G1NYqeL) have been replaced with truncated placeholders like TDqSq...xxxxx and TNPee...xxxxx. While this reduces exposure of real addresses (good intent), the placeholder format is inconsistent — some use ...xxxxx, some use TSpender..., TRecipientAddress, TFriendAddress. This inconsistency makes the examples harder to parse and may confuse users about what format to use.

Recommendation:

  • Standardize placeholder format across all docs — e.g., use T<Name>... (like TRecipient...) consistently for named roles and Txxx...xxx for opaque addresses.
  • Alternatively, use clearly fictional TRON-format addresses (TRON addresses are 34 chars starting with T) to make the examples copy-paste friendly in testing contexts.

[severity-10] Redundant Whitespace in QuickStart.md

Property Value
Severity Suggestion
Category Quality
File docs/Openclaw-extension/QuickStart.md : Line ~433 (after BANKOFAI_API_KEY table)

Description: A blank line was added after the BANKOFAI_API_KEY table row — two consecutive blank lines appear in the source. While Markdown rendering is unaffected, it is inconsistent with the document's style.

Recommendation: Remove the extra blank line to maintain consistent formatting.


[severity-11] recharge-skill Listed with Alert in One Security Table Only

Property Value
Severity Suggestion
Category Docs
File docs/Openclaw-extension/QuickStart.md : Line 330

Description: In the Openclaw QuickStart, recharge-skill is shown with Safe / 1 alert / Med Risk — but in the SKILLS QuickStart it shows -- / -- / --. If the "1 alert" and "Med Risk" rating for recharge-skill is accurate, users reading only the SKILLS guide are underinformed about this skill's risk profile.

Recommendation: Either show the alert in both docs, or if the rating has changed since the SKILLS doc was last updated, update both simultaneously.


[severity-12] Section Heading Style Change May Break Anchor Links

Property Value
Severity Suggestion
Category Quality / Correctness
File docs/McpServer-Skills/SKILLS/BANKOFAISkill.md : Lines 62, 94, 122, 222

Description: Section headings were simplified, removing the descriptive subtitle portion:

  • ## sunswap — Swap Tokens, Check Prices, Manage Pools {#sunswap}## sunswap {#sunswap}
  • ## sunperp-skill — Perpetual Contract Trading {#sunperp-skill}## sunperp-skill {#sunperp-skill}

The anchor IDs ({#sunswap}, {#sunperp-skill}, etc.) are preserved, so internal links using #sunswap will continue to work. However, any external links or bookmarks that used auto-generated anchors from the verbose headings (e.g., #sunswap--swap-tokens-check-prices-manage-pools) would break. This is a low risk if the Docusaurus config mandates explicit IDs, which appears to be the case here.

Recommendation: Verify that no external or cross-document links relied on the old auto-generated anchors. Since explicit {#id} anchors are used, this is likely safe — but a quick grep for the old slugs across the repo is worth doing.


4. Positive Observations

  1. Consistent dual-language maintenance: Every English doc change is mirrored in the i18n/zh-Hans/ counterpart. This is disciplined and important for a bilingual documentation site.

  2. Address masking is a good security practice: Replacing real blockchain addresses in example prompts and documentation reduces the risk of those addresses being targeted or mistaken for correct test addresses.

  3. Clear Windows security disclosure: The documentation explicitly discloses that BNB Chain stores private keys in plaintext (mcporter.json), recommends a dedicated wallet with minimal funds, and explains the icacls ACL mechanism used as equivalent to chmod 600. This level of transparency is commendable.

  4. Step-by-step install wizard documentation: The new QuickStart.md for Openclaw-extension is significantly more detailed — it documents every interactive prompt with exact expected output. This dramatically reduces support burden and user confusion.

  5. Double-confirmation for destructive "clean install" mode: The installer requires the user to type CLEAN to confirm permanent deletion of all wallet data, a solid UX safeguard against accidental data loss.

  6. New skills clearly scoped by read-only vs write operations: The trc20-toolkit-skill and multisig-permissions documentation clearly separates "read-only, safe" operations from "requires your confirmation" operations, which helps users understand what is irreversible.

  7. Explicit InsufficientBalanceError clarification in SDK Guide: The added tip clarifying that InsufficientBalanceError is not thrown by the SDK itself, but is provided for the caller's use, is exactly the kind of documentation that prevents hours of debugging.

  8. Security tips section added at end of Openclaw QuickStart: The new ## Security Tips section consolidates key security advice (dedicated wallet, testnet first, master password importance) in one easy-to-find location.


5. Checklist Results

Category Check Status Notes
Correctness Default wallet ID updated consistently PASS defaultdefault_secure applied in all relevant docs
Correctness Skill count updated (6 → 7) PASS Applied in all three QuickStart docs
Correctness Address examples masked PASS Real addresses replaced with placeholders
Correctness Node.js v18 EOL risk FAIL v18 is EOL since Apr 2025; docs now recommend an unsupported version
Correctness API base URL migration FAIL Breaking URL change undocumented for existing users
Correctness Python removal rationale FAIL No explanation for dependency removal
Security BNB private key risk disclosed PARTIAL Disclosed, but placement relative to prompt could be improved
Security `irm iex` risk acknowledged FAIL
Security Feature branch in Docker push trigger FAIL ai-bankofai-patch-1 triggers Docker Hub push
Security Sensitive data in docs PASS No actual secrets or full private keys in docs
Performance N/A (docs-only PR) PASS
Quality Section headings simplified (anchors preserved) PASS Explicit {#id} anchors used throughout
Quality Version skip (1.2.4 → 1.2.6) MINOR Missing explanation for skipped patch version
Quality Security tables inconsistent FAIL recharge-skill and x402-payment ratings differ between docs
Quality Placeholder address format inconsistent PARTIAL Mix of ...xxxxx and named placeholders like TRecipientAddress
Testing N/A (docs-only PR, no code tests) N/A
i18n All changes mirrored in zh-Hans PASS All 11 i18n files updated correspondingly
Docs New skills fully documented PASS Both trc20-toolkit-skill and multisig-permissions complete
Docs Windows support documented end-to-end PASS Install, config, FAQ, paths, permissions all covered

6. Review Verdict

Decision: Request Changes

This PR is a substantial and well-structured documentation update that adds Windows support, two new skills, and improved user-facing content. The i18n parity is excellent, and the depth of the installer walkthrough is impressive. However, there are issues that should be resolved before merging:

Must Fix (blocking):

  1. [severity-01] The base_url change in bankofai-config.json is a silent breaking change for existing users — a migration note is required.
  2. [severity-03] Documenting Node.js v18 (EOL since April 2025) as the minimum version is a security regression — must be corrected to v20 or explained with a clear caveat.
  3. [severity-04] The ai-bankofai-patch-1 branch should be removed from the Docker push trigger in the CI workflow to prevent unintended image publishing.

Should Fix (non-blocking but recommended):
4. [severity-02] Improve the BNB Chain plaintext key warning placement.
5. [severity-05] Synchronize security risk tables across QuickStart documents.
6. [severity-08] Add a brief irm | iex security acknowledgment in the Windows install section.

Once the three blocking issues are resolved and the non-blocking items are addressed or acknowledged, this PR is suitable for approval.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 1, 2026

Code Review Report

Project: @x402-tron/docs (BofAI Documentation)
PR: mainupdate-mcp-server
Review Date: 2026-04-01
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)


PR Overview

Branch Information

Property Value
From Branch origin/main
To Branch origin/update-mcp-server
Commits 25
Files Changed 24
Lines Added +1,232
Lines Removed -158

Commit History

Hash Message
676b506 update Description
930cbcc Address Masking
7f26fd6 update agent-wallet and skills
bfba297 update openclaw-extension
d27f93f update openclaw extension
b85f8e1 update setup
781a1f4 Merge pull request #39 from BofAI/ai-bankofai-patch-1
bfb5e6a add windows setup
1594cb7 update version
e03b329 Update API.md
(+ 15 more) (earlier history from merged branch)

Review Summary

Verdict

Verdict: Request Changes

Findings at a Glance

Critical Major Minor Suggestion
Count 0 4 3 4

Summary

This PR is a large documentation-only update that adds Windows platform support and two new skills (trc20-toolkit-skill and multisig-permissions) across English and Chinese documentation. The changes include updated install wizard walkthroughs, expanded FAQ sections, revised Intro pages, and Agent-Wallet clarifications. There are no application code changes — package.json only has a version bump (1.2.4 → 1.2.6) and .github/workflows/docker.yml has a CI trigger addition.

No critical security vulnerabilities or data-loss bugs were found in the changed code. However, four major issues require attention before merging: a development branch leaked into permanent CI/CD triggers, a potentially breaking default wallet ID rename with no migration guidance, a silent API base URL change that will silently break existing BofAI recharge configurations, and the continued documentation of plaintext private key storage for BNB Chain without stronger mitigations. Three minor issues cover remote script execution practices, a documentation discrepancy in the security scan table, and a Node.js version messaging inconsistency.


Change Summary

1. CI/CD Configuration

File Change Type Description
.github/workflows/docker.yml Modified Adds ai-bankofai-patch-1 to push and pull_request CI triggers

Purpose: Enables Docker CI builds on the ai-bankofai-patch-1 branch — appears to be a development convenience that was not removed before merging.


2. Agent-Wallet Documentation (EN + ZH)

File Change Type Description
docs/Agent-Wallet/Developer/CLI-Reference.md Modified Clarifies password retry behavior, confirmation flow, and default wallet IDs
docs/Agent-Wallet/Developer/SDK-Guide.md Modified Adds tip clarifying InsufficientBalanceError is not thrown by SDK
docs/Agent-Wallet/QuickStart.md Modified Updates default wallet ID in CLI example from defaultdefault_secure
i18n/zh-Hans/... (3 files) Modified Chinese mirrors of the above

Purpose: Documents behavior changes to the AgentWallet CLI: default wallet ID renamed, password retry limit (3 attempts), and confirmation flow for new passwords.


3. New Skills Documentation (EN + ZH)

File Change Type Description
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md Modified Adds full sections for trc20-toolkit-skill and multisig-permissions
docs/McpServer-Skills/SKILLS/Intro.md Modified Updates count "Five" → "Seven"; adds two new skill intro sections
docs/McpServer-Skills/SKILLS/QuickStart.md Modified Updates skill count 6 → 7; adds TRC20 Token Toolkit to install output
docs/McpServer-Skills/SKILLS/Faq.md Modified Adds trc20-toolkit-skill and multisig-permissions to expected skill directories
i18n/zh-Hans/... (4 files) Modified Chinese mirrors of the above

Purpose: Documents two newly available skills; updates all references to reflect the new skill count.


4. Openclaw Extension — Windows Support & Expanded Walkthrough (EN + ZH)

File Change Type Description
docs/Openclaw-extension/QuickStart.md Modified Major rewrite: adds Windows tab (PowerShell install), detailed per-step walkthroughs, security tips, config file reference table
docs/Openclaw-extension/Intro.md Modified Restructured intro; adds AgentWallet as first core component; adds Windows support mention
docs/Openclaw-extension/FAQ.md Modified Replaces Python FAQ with Git FAQ; adds Windows-specific section
i18n/zh-Hans/... (3 files) Modified Chinese mirrors of the above

Purpose: Full Windows 10/11 support added to the installer (v1.4.5); documentation updated to match. Node.js requirement lowered from v20 to v18.


5. Address Masking

File Change Type Description
docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md Modified Real TRON address TNPeeaaFB7K9cmo4uQpcU32zGK8G1NYqeL replaced with placeholder TNPee...xxxxx
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md Modified Real TRON address TDqSquXBgUCLYvYC4XZgrprLK589dkhSCf replaced with placeholder TDqSq...xxxxx
i18n/zh-Hans/... (2 files) Modified Chinese mirrors of the above

Purpose: Removes real third-party TRON addresses from documentation examples to prevent confusion or misuse.


6. Package Version Bump

File Change Type Description
package.json Modified Version bumped from 1.2.41.2.6

Detailed Findings


Major

[MJ-01] Feature Branch Leaked Into Permanent CI/CD Triggers

Property Value
Severity Major
Category Correctness / Compatibility
File .github/workflows/docker.yml : Lines 9, 17

Description

The development branch ai-bankofai-patch-1 has been added to both the push and pull_request trigger lists in the Docker CI workflow. This branch appears to be a temporary feature/patch branch (it is also listed in the remote branch list). Permanently encoding a transient feature branch name into a shared CI workflow causes:

  • CI resource waste: Every future push to any branch named ai-bankofai-patch-1 will trigger a Docker build, even from unrelated forks.
  • CI signal noise: Stale branch triggers can generate false-positive build notifications.
  • Precedent risk: Opening the door to accumulating more stale branch entries over time.

Code

on:
  push:
    branches:
      - main
      - master
      - update-mcp-server
+     - ai-bankofai-patch-1   # ← feature branch; should not be permanent
  pull_request:
    branches:
      - main
      - master
      - update-mcp-server
+     - ai-bankofai-patch-1   # ← same issue

Recommendation

Remove both ai-bankofai-patch-1 entries before merging. If CI builds are needed during development of a feature branch, use workflow_dispatch (already present) or a temporary branch-specific workflow file instead.

on:
  push:
    branches:
      - main
      - master
      - update-mcp-server
  pull_request:
    branches:
      - main
      - master
      - update-mcp-server
  workflow_dispatch:

[MJ-02] Default Wallet ID Rename (defaultdefault_secure) Is a Silent Breaking Change

Property Value
Severity Major
Category Compatibility / Documentation
File docs/Agent-Wallet/QuickStart.md : Lines 78–99; docs/Agent-Wallet/Developer/CLI-Reference.md : Lines 34–35

Description

The documented default wallet ID for local_secure wallets changed from default to default_secure, and for raw_secret wallets from default_raw to default_raw (unchanged). The CLI Reference docs confirm: "If you don't specify a wallet ID, the system defaults to default_secure for local_secure wallets."

Any existing user who initialized AgentWallet under the old default (default) will have their wallet effectively deactivated after upgrading the CLI, because the new CLI's auto-selection will look for default_secure and not find it. This is a breaking change with no migration guidance anywhere in this PR.

Code

-Wallet ID (e.g. my_wallet_1) (default):
+Wallet ID (e.g. my_wallet_1) (default_secure):

-│ default   │ local_secure │
+│ default_secure │ local_secure │

-Active wallet: default
+Active wallet: default_secure

Recommendation

Add an upgrade notice (e.g., a Docusaurus :::warning admonition) in the QuickStart and CLI-Reference pages explaining:

  1. If upgrading from a version that created a default wallet, rename it: agent-wallet rename default default_secure (or equivalent command).
  2. Alternatively, always pass -w default explicitly until the wallet is renamed.

If the CLI provides a migration command, link to it. If not, this should be tracked as a follow-up action.


[MJ-03] base_url Change in BANK OF AI Config Breaks Existing Installations Silently

Property Value
Severity Major
Category Compatibility / Correctness
File docs/Openclaw-extension/QuickStart.md : Lines 1025–1026

Description

The one-liner configuration command for the BANK OF AI API key silently changes the base_url value:

  • Before: https://api.bankofai.io/v1/
  • After: https://chat.bankofai.io/

Any user who already has ~/.mcporter/bankofai-config.json with the old URL will not be automatically migrated. If these are two different endpoints (different path, no trailing /v1/), the recharge-skill will silently fail or send requests to the wrong endpoint after following the new docs.

Code

-mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://api.bankofai.io/v1/"}' > ~/.mcporter/bankofai-config.json
+mkdir -p ~/.mcporter && echo '{"api_key": "...", "base_url": "https://chat.bankofai.io/"}' > ~/.mcporter/bankofai-config.json

Recommendation

  1. Add a migration note explaining the URL change and that existing users need to update their config file.
  2. If the old URL still works (redirects or is kept alive), document it explicitly.
  3. Consider whether the installer script should auto-update existing config files when run in Normal install mode.

[MJ-04] BNB Chain Private Key Stored in Plaintext — Insufficient Risk Mitigation in Docs

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 828–840

Description

The documentation explicitly states that the BNB Chain MCP server (bnbchain-mcp) stores the user's private key in plaintext in ~/.mcporter/mcporter.json. The existing caution block advises using a "dedicated wallet with minimal funds," but this is insufficient risk mitigation for a blockchain private key:

  • A compromised mcporter.json (e.g., via a malicious MCP skill, a path traversal bug, or simple file sharing) immediately exposes all funds.
  • chmod 600 protection (or icacls on Windows) is only effective against other local users, not against processes running as the same user — including the AI agent itself.
  • There is no mention of when or whether AgentWallet integration is planned for BNB Chain.

Code

⚠ Your PRIVATE_KEY will be stored in plaintext in: ~/.mcporter/mcporter.json

? Enter BNB Chain PRIVATE_KEY (optional):

Recommendation

The documentation should:

  1. Upgrade the caution level — use :::danger instead of :::caution for this specific warning.
  2. Add explicit guidance: use a throwaway wallet with zero balance until AgentWallet encryption is supported, or skip this step entirely.
  3. Add a roadmap note: "BNB Chain AgentWallet integration is planned for version X.Y" (if known), so users understand the current limitation is temporary.
  4. Remove the PRIVATE_KEY entry from the "Configuration File Reference" table's description without a matching warning annotation.

Minor

[MN-01] Remote Script Execution Without Integrity Verification

Property Value
Severity Minor
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines 601, 619

Description

Both primary installation methods — curl -fsSL ... | bash (Linux/macOS) and irm ... | iex (Windows) — download and immediately execute a remote script with no integrity check. If GitHub raw content delivery or the repository is ever compromised, users could unknowingly execute malicious code. This is a known security pattern concern ("curl-to-bash").

Code

curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bash
irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

Recommendation

Add an optional (but recommended) checksum verification step to the docs:

# Verify SHA256 before executing (recommended)
curl -fsSL https://raw.githubusercontent.com/BofAI/.../install.sh -o install.sh
echo "<expected_sha256>  install.sh" | sha256sum -c && bash install.sh

At minimum, add a note advising users to review the script content before running it, and link to the GitHub source for manual inspection.


[MN-02] Security Risk Assessment Table Inconsistency Between Old and New Docs

Property Value
Severity Minor
Category Documentation
File docs/McpServer-Skills/SKILLS/QuickStart.md : Lines 91–104; docs/Openclaw-extension/QuickStart.md : Lines 896–905

Description

The security scan results table in the installer output example was updated, but the changes are not explained. Specifically:

Skill Old Risk Level New Risk Level
recharge-skill -- -- -- (all dashes) Safe / 1 alert / Med Risk
x402-payment Med / 1 alert / Med Safe / 0 alerts / Med Risk

These are documented example outputs — if they reflect a real change in the underlying security scan, the reason should be noted. If the numbers changed because the scanner was re-run and got different results, users may be confused about why a skill previously shown as low-risk now shows "Med Risk."

Recommendation

Either:

  1. Add a brief footnote explaining that scan results reflect the state at documentation time and may vary, or
  2. If these are intentional updates to reflect new scan runs, add a note in the changelog linking to the relevant scan report.

[MN-03] Node.js Version Requirement Message May Be Misleading

Property Value
Severity Minor
Category Documentation
File docs/Openclaw-extension/QuickStart.md : Line 572; docs/Openclaw-extension/FAQ.md : Line 384

Description

The Node.js minimum requirement was lowered from v20 to v18 (consistent with package.json's "node": ">=18"). However, the documentation still states "older versions will definitely cause errors!" immediately after stating v18 is required. This phrasing, combined with the FAQ answer that now recommends "v18 or higher" (changed from v20), may confuse users who have v18 or v19 — are those "older" versions that will cause errors?

Code

-2. **Node.js** (must be v20 or above): ... *(Extremely important — older versions will definitely cause errors!)*
+2. **Node.js** (must be v18 or above): ... *(Extremely important — older versions will definitely cause errors!)*

Recommendation

Clarify the warning to be specific:

Node.js (v18 or above required): The runtime environment for skill packs and configuration. (v17 and below are not supported and will cause errors.)


Suggestions

[S-01] CI Branch Hygiene — Establish a Policy for Temporary Branch Triggers

File: .github/workflows/docker.yml
Description: There is no documented process for adding/removing branches from CI triggers. The accidental inclusion of ai-bankofai-patch-1 suggests a process gap.
Suggestion: Add a comment in docker.yml listing which branches are permanent vs. temporary, and add a PR checklist item to review CI trigger lists before merging.


[S-02] Wallet Migration Documentation for Existing Users

File: docs/Agent-Wallet/QuickStart.md, docs/Agent-Wallet/Developer/CLI-Reference.md
Description: The default wallet ID rename from defaultdefault_secure is an undocumented breaking change.
Suggestion: Create or link to an upgrade guide in the AgentWallet section. Even a single admonition block noting "If you installed before version X.Y, your active wallet may be named default — run agent-wallet set-active default to restore it" would significantly reduce user confusion.


[S-03] Add a BNB Chain AgentWallet Roadmap Note

File: docs/Openclaw-extension/QuickStart.md
Description: The plaintext key storage for BNB Chain is a known limitation. Users would benefit from knowing when encrypted storage is expected.
Suggestion: Add a :::info or :::tip block: "AgentWallet support for BNB Chain is on our roadmap. Until then, please use a dedicated wallet with minimal funds." This frames the limitation as temporary and keeps users informed.


[S-04] "Clean Install" Destructive Action — Stronger Warning in Docs

File: docs/Openclaw-extension/QuickStart.md : Lines 658–710
Description: The "Clean install" option deletes all wallet data including runtime_secrets.json, wallets_config.json, etc. The confirmation flow (type "CLEAN") is documented, but the consequences of wallet data deletion (loss of access to on-chain assets if master password was not saved) are mentioned only lightly.
Suggestion: Add a prominent :::danger admonition before the Clean install section: "Clean install permanently deletes your AgentWallet data. If you have funds in your AI agent wallet and do not have your master password saved, you will lose access to those funds. Back up your master password before proceeding."


Positive Observations

Area Observation
Address Privacy Real TRON addresses (TNPeeaaFB7K9cmo4uQpcU32zGK8G1NYqeL, TDqSquXBgUCLYvYC4XZgrprLK589dkhSCf) were replaced with truncated placeholders — a good proactive step to avoid unwanted on-chain activity.
Confirmation Flow UX The Clean install double-confirmation pattern (y/N + type CLEAN) is a solid safeguard against accidental destructive operations.
Security Tips Section The new dedicated "🔒 Security Tips" section in QuickStart.md consolidates security guidance in one place — easy to find and follow.
Windows FAQ Coverage The new Windows-specific FAQ section covers common failure modes (execution policy, garbled characters, WSL caveats) thoroughly.
InsufficientBalanceError Clarification The SDK docs correctly clarify that this error is a user-space convenience type — not thrown by the SDK — preventing developer confusion.
Password Retry Behavior Documenting the 3-attempt limit for local_secure wallet sign commands sets clear expectations and reduces support load.
i18n Parity All English changes are fully mirrored in Chinese (i18n/zh-Hans), maintaining complete bilingual coverage.
Tabbed Platform Docs Using <Tabs> (Docusaurus) for macOS/Linux vs. Windows platform split is the right UX pattern — avoids content duplication while keeping instructions clear per platform.

Checklist Results

Category Items Checked Pass Fail N/A Notes
Correctness 8 6 2 0 MJ-02 (wallet ID rename), MJ-03 (base_url change)
Security 10 7 3 0 MJ-04 (plaintext key), MN-01 (curl|bash), S-04 (clean install warning)
Performance 7 - - 7 No performance-relevant code changed
Code Quality 10 9 1 0 MJ-01 (CI branch leak)
Testing 7 - - 7 Documentation-only PR
Documentation 6 4 2 0 MN-02 (scan table discrepancy), MN-03 (Node version messaging)
Compatibility 5 3 2 0 MJ-02 (wallet ID breaking), MJ-03 (base_url breaking)
Observability 4 - - 4 No application code changed

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, and deployment impact are not covered. The security findings related to BNB Chain private key storage and remote script execution reflect documentation practices; the actual installer code was not reviewed in this audit.


Report generated by Code Review Skill v1.0.0
Date: 2026-04-01

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 1, 2026

Code Review Report

Project: BofAI / openclaw-extension Docs
PR: mainupdate-mcp-server
Review Date: 2026-04-01
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)


PR Overview

Branch Information

Property Value
From Branch origin/main
To Branch origin/update-mcp-server
Commits 23
Files Changed 24
Lines Added +1,232
Lines Removed -158

Commit History (most recent first)

Hash Message
42d7555 masking tron address
676b506 update Description
930cbcc Address Masking
7f26fd6 update agent-wallet and skills
bfba297 update openclaw-extension
d27f93f update openclaw extension
b85f8e1 update setup
781a1f4 Merge pull request #39 from BofAI/ai-bankofai-patch-1
bfb5e6a add windows setup
1594cb7 update version
e03b329 Update API.md
(+ 12 earlier commits) various updates and merges

Review Summary

Verdict

Verdict: Request Changes

Changes are needed primarily around security guidance for installation commands, a silent breaking change to the BANKOFAI API base URL, and a Node.js minimum version that now recommends an end-of-life release.

Findings at a Glance

Critical Major Minor Suggestion
Count 0 2 4 3

Summary

This PR is a documentation-only release — no application source code was changed. It delivers two major content themes: (1) Windows support documentation for the OpenClaw Extension installer, and (2) two new MCP skills (trc20-toolkit-skill and multisig-permissions). All English documentation has a parallel Chinese (zh-Hans) counterpart that receives identical updates, which is good practice.

The writing quality is high throughout; the step-by-step wizard walkthroughs, tabbed Mac/Linux vs. Windows instructions, and security-focused callouts (BNB Chain plaintext key warning, master password caution) all represent genuine improvements for end users.

However, two major issues need attention before merge: the installation commands recommend piping a remote script directly to a shell interpreter without integrity verification — a known risky pattern — and the BANKOFAI API base URL silently changed in a user-facing command without any migration guidance. Additionally, the minimum Node.js version was lowered to a release that is now end-of-life.


Change Summary

1. CI / Build — Trigger Expansion

File Change Type Description
.github/workflows/docker.yml Modified Added ai-bankofai-patch-1 to push and pull_request branch triggers

Purpose: Enable the Docker build workflow to run automatically on the ai-bankofai-patch-1 development branch, mirroring existing branches.


2. Agent-Wallet Documentation Corrections

File Change Type Description
docs/Agent-Wallet/Developer/CLI-Reference.md Modified Added password retry/confirmation behavior and default wallet ID defaults
docs/Agent-Wallet/Developer/SDK-Guide.md Modified Clarified that InsufficientBalanceError is not thrown by the SDK itself
docs/Agent-Wallet/QuickStart.md Modified Updated default wallet ID from default to default_secure in example output
(zh-Hans mirrors of the above) Modified Same corrections in Chinese

Purpose: Align documentation with apparent behavior changes in the AgentWallet CLI: the default wallet ID changed from defaultdefault_secure, and password retry limits (3 total attempts) and confirmation flow are now explicitly documented.


3. New Skills — trc20-toolkit-skill & multisig-permissions

File Change Type Description
docs/McpServer-Skills/SKILLS/BANKOFAISkill.md Modified Added full documentation sections for both new skills; masked example TRON addresses
docs/McpServer-Skills/SKILLS/Intro.md Modified Updated skill count 5→7; added entry sections for both new skills
docs/McpServer-Skills/SKILLS/QuickStart.md Modified Updated installer output samples (6→7 skills, added TRC20 Token Toolkit to tables)
docs/McpServer-Skills/SKILLS/Faq.md Modified Updated skill directory listing
docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md Modified Masked live TRON address in example
(zh-Hans mirrors of the above) Modified Same in Chinese

Purpose: Document two newly available skills and sanitize real TRON addresses used as documentation examples.


4. Openclaw Extension — Windows Support & Installer Walkthrough

File Change Type Description
docs/Openclaw-extension/Intro.md Modified Major rewrite: restructured from 2-weapon to 3-weapon model (added AgentWallet), added Windows mention
docs/Openclaw-extension/QuickStart.md Modified +442 lines: added tabbed Mac/Linux vs Windows install commands, step-by-step wizard walkthrough, config file reference, security tips section; changed Node.js min v20→v18, removed Python 3 prerequisite; updated BANKOFAI API base URL
docs/Openclaw-extension/FAQ.md Modified Replaced Python 3 FAQ with Git FAQ; added Windows-specific FAQ section (execution policy, garbled output, admin privileges, WSL)
(zh-Hans mirrors of the above) Modified Same in Chinese

Purpose: Add Windows 10/11 installation support documentation for the OpenClaw Extension installer (install.ps1 / install.bat), expand the installation guide from a brief overview to a detailed step-by-step walkthrough, and clean up prerequisites.


5. Package Version Bump

File Change Type Description
package.json Modified Version bumped from 1.2.41.2.6

Detailed Findings


Major

[MJ-01] Piping Remote Scripts Directly to Shell Interpreter Without Integrity Verification

Property Value
Severity Major
Category Security
File docs/Openclaw-extension/QuickStart.md : Lines ~38–52 (English); i18n/zh-Hans/.../QuickStart.md : equivalent section

Description

The PR introduces and promotes two "one-liner" installation commands — one for Linux/macOS and one for Windows — that download and immediately execute a remote script without any integrity check:

# Linux/macOS
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.sh | bash

# Windows
irm https://raw.githubusercontent.com/BofAI/openclaw-extension/refs/heads/main/install.ps1 | iex

Both patterns have the same fundamental risk: if the GitHub repository or CDN is compromised (supply-chain attack, branch hijack, DNS poisoning), the user's machine executes attacker-controlled code with their full user-level privileges. The scripts also install agent-wallet which stores encrypted private keys — making the target especially sensitive. The documentation is actually quite good elsewhere about security (BNB plaintext key warning, minimal-funds advice), so this gap stands out.

Recommendation

Add a checksum/hash verification step or, at a minimum, recommend the "install from source" alternative (already shown in the PR) as the preferred option for security-conscious users:

> **Security-conscious users**: We recommend the "install from source" approach — clone
> the repository, inspect `install.sh` / `install.ps1`, then run it locally. The
> one-liner is provided for convenience only.

For the one-liner path, consider pinning to a specific commit SHA instead of refs/heads/main (a mutable ref) and publishing a SHA-256 checksum alongside:

# Pin to a specific, immutable commit (example)
curl -fsSL https://raw.githubusercontent.com/BofAI/openclaw-extension/<commit-sha>/install.sh | bash
# Verify: sha256sum install.sh  →  <expected-hash>

[MJ-02] BANKOFAI API Base URL Changed Without Migration Guidance

Property Value
Severity Major
Category Correctness / Documentation
File docs/Openclaw-extension/QuickStart.md : "Configure BANK OF AI" section

Description

The one-click command to create bankofai-config.json silently changed the base_url value:

- {"api_key": "...", "base_url": "https://api.bankofai.io/v1/"}
+ {"api_key": "...", "base_url": "https://chat.bankofai.io/"}

Existing users who set up their config with the old URL (following the previous docs or their own notes) will now be running with a stale endpoint. There is no callout, migration note, or upgrade instruction anywhere in the PR. If the old URL is deprecated or returning errors, users will not know why their recharge or balance checks suddenly fail.

Recommendation

Add a visible callout near this command explaining the URL change:

:::caution If you already have a config file
The BANK OF AI API base URL has changed. Update your existing
`~/.mcporter/bankofai-config.json` (or Windows equivalent) and change
`base_url` to `"https://chat.bankofai.io/"`.
:::

Additionally, include this as a note in the FAQ.md under a "Upgrading" section.


Minor

[MN-01] Node.js Minimum Version Lowered to an End-of-Life Release

Property Value
Severity Minor
Category Compatibility / Documentation
File docs/Openclaw-extension/QuickStart.md (Lines ~9–10); docs/Openclaw-extension/FAQ.md (Line ~12); both zh-Hans mirrors

Description

The PR lowers the documented Node.js minimum from v20 to v18:

- Node.js (must be v20 or above)
+ Node.js (must be v18 or above)

Node.js 18 reached End of Life in April 2025. As of this PR's date (April 2026), Node 18 no longer receives security patches. Recommending an EOL version could lead users to run on insecure, unpatched software, which is especially problematic given the crypto-wallet nature of this project.

Recommendation

Keep the minimum at v20 LTS (supported until April 2026) or raise it to v22 LTS (supported until April 2027):

- Node.js (must be v18 or above)
+ Node.js (must be v20 or above, v22 LTS recommended)

Update all FAQ and QuickStart occurrences consistently.


[MN-02] Security Risk Assessment Shows Unexplained "Med Risk" for recharge-skill

Property Value
Severity Minor
Category Security / Documentation
File docs/McpServer-Skills/SKILLS/QuickStart.md : Security Risk Assessments table; zh-Hans mirror

Description

The security risk table now shows recharge-skill with a Safe / 1 alert / Med rating:

│  recharge-skill                    Safe       1 alert       Med Risk  │
│  x402-payment                      Safe       0 alerts      Med Risk  │

Previously only x402-payment appeared with a warning. The table is reproduced as documentation for users without any explanation of what the "1 alert" in recharge-skill is, why it's rated Med Risk, and whether it is acceptable. A user following the wizard will see this and have no guidance on how to interpret or respond to it.

Recommendation

Add a note below the table explaining what the alerts mean and why they are considered acceptable (or document any mitigating factors):

:::note About the security ratings
`recharge-skill` has 1 Socket alert related to [explain reason]. This is expected
because [context]. Both `recharge-skill` and `x402-payment` are rated Med Risk due
to [reason], which is inherent to any skill that can initiate on-chain transactions.
:::

[MN-03] Version Jumped from 1.2.4 to 1.2.6, Skipping 1.2.5

Property Value
Severity Minor
Category Code Quality
File package.json : Line 3

Description

- "version": "1.2.4",
+ "version": "1.2.6",

A version number was skipped. While this is not a functional bug in a documentation project, it can confuse changelog tracking, automated tooling, or users looking for a 1.2.5 release. The commit history shows update version as a separate commit, suggesting this was a deliberate or accidental double bump.

Recommendation

Confirm whether 1.2.5 was intentionally skipped (e.g., if it was published and yanked). If not intentional, correct to 1.2.5. If intentional, add a comment or CHANGELOG note explaining the skip.


[MN-04] CI Trigger Added for a Feature Branch That May Be Transient

Property Value
Severity Minor
Category Compatibility / Maintenance
File .github/workflows/docker.yml : Lines 9, 17

Description

+      - ai-bankofai-patch-1

ai-bankofai-patch-1 was added to both push and pull_request CI triggers. This appears to be a developer feature branch. Adding transient feature branches to the central workflow creates noise (every push to that branch triggers a full Docker build) and will leave a dead trigger once the branch is merged and deleted.

Recommendation

Remove ai-bankofai-patch-1 from the CI branch list before merging this PR, or use a wildcard pattern if the intent is to support all patch branches:

branches:
  - main
  - master
  - update-mcp-server
  # Remove ai-bankofai-patch-1 — transient dev branch

Suggestions

[S-01] Pin Install Script to Immutable Commit SHA Instead of Branch Head

File: docs/Openclaw-extension/QuickStart.md, i18n/zh-Hans/.../QuickStart.md
Description: Both one-liner install commands reference refs/heads/main — a mutable ref. If main is updated between when a user reads the docs and runs the command, they may get a different version than documented.
Suggestion: Pin to a specific commit SHA or tag that matches the documented version, and update with each release. This also makes reproducibility easier and supports the supply-chain integrity point raised in MJ-01.


[S-02] Provide Guidance on Rotating BNB Chain Private Key

File: docs/Openclaw-extension/QuickStart.md — BNB Chain Toolbox section
Description: The PR clearly warns that the BNB Chain private key is stored in plaintext and recommends using a "dedicated wallet with minimal funds." This is good, but there's no guidance on how to rotate or update the key if it is suspected to be compromised.
Suggestion: Add a brief note or link explaining how to update the mcporter.json entry to change the BNB Chain key.


[S-03] Inconsistent Address Masking Style

File: docs/McpServer-Skills/SKILLS/BANKOFAISkill.md, docs/McpServer-Skills/MCP/TRONMCPServer/LocalPrivatizedDeployment.md
Description: Real TRON addresses are masked using two different styles: TNPee...xxxxx (5 chars + ellipsis + xxxxx) and TDqSq...xxxxx (5 chars + ellipsis + xxxxx) — these are consistent. However, placeholder addresses in the new skill sections use plain descriptive names like TRecipientAddress, TSpenderAddress, and TVendor... without the ...xxxxx suffix.
Suggestion: Standardize all example addresses to one format (e.g., TXxxxx...xxxxx) to avoid readers mistaking descriptive placeholder names for real addresses.


Positive Observations

Area Observation
Security transparency The BNB Chain plaintext private key limitation is prominently disclosed with a :::caution admonition and a clear recommendation to use a dedicated minimal-funds wallet. This is excellent security communication.
Windows parity The Windows installer documentation is thorough: covers execution policy bypass, garbled terminal output, WSL trade-offs, and admin privilege requirements. This shows careful testing on the target platform.
Address privacy Proactively masking real TRON addresses from documentation examples (e.g., TNPeeaaFB7K9cmo4uQpcU32zGK8G1NYqeLTNPee...xxxxx) is good privacy practice and prevents users from accidentally using test addresses.
Dual-language parity Every English documentation change has an exact Chinese counterpart in i18n/zh-Hans/. The translations are consistent and appear accurate.
InsufficientBalanceError clarification The SDK Guide correction explaining that InsufficientBalanceError is never thrown by the SDK itself (it's provided for caller use) prevents a common developer misconception.
Multi-skill documentation The new trc20-toolkit-skill and multisig-permissions sections follow a consistent structure (safe/read-only operations vs. confirmation-required operations vs. real-world scenarios) that is easy to scan and understand.
Step-by-step wizard walkthrough The QuickStart guide now shows actual terminal output at each step, removing ambiguity about what users should see and dramatically reducing support burden.
Clean install safety The Clean install flow requires users to type CLEAN to confirm permanent deletion — a good guard against accidental data loss.

Checklist Results

Category Items Checked Pass Fail N/A Notes
Correctness 5 3 1 1 API base URL change lacks migration note (MJ-02)
Security 8 5 2 1 curl/irm pipe-to-shell (MJ-01); EOL Node.js (MN-01)
Performance 7 0 0 7 No performance-sensitive code changed
Code Quality 6 4 2 0 Version skip (MN-03); CI transient branch (MN-04)
Testing 7 0 0 7 Documentation-only PR; no tests applicable
Documentation 6 5 1 0 Unexplained security alert (MN-02)
Compatibility 5 3 1 1 Node.js EOL version (MN-01)
Observability 4 0 0 4 No application code changed

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. This PR contains exclusively documentation changes — no application logic, cryptographic code, or smart contracts were modified. Runtime behavior, integration testing, and deployment impact are not covered.


Report generated by Code Review Skill v1.0.0
Date: 2026-04-01

@Will-Guan Will-Guan merged commit 1d5c13a into main Apr 1, 2026
4 checks passed
@github-actions github-actions Bot mentioned this pull request Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants