fix(weixin-bridge): make the Weixin bridge runnable and simplify its Quick Start - #6170
VincentCorleone wants to merge 2 commits into
Conversation
Define a Quick Start section in integrations/weixin-bridge/README.md modeled on integrations/wecom-bridge/DEPLOYMENT.md, and then implement the behavior that section describes. The README previously pointed at a non-existent /opt/codewhale/weixin-bot-bridge path, gave no runtime startup command, and named an env file that nothing reads; a first-time user could not get the bridge running, and once it did run the first incoming message failed with EACCES and was dropped without an actionable error. README: - Add Quick Start with the two-terminal flow (start `codewhale serve --http`, then the bridge) and a single-terminal launcher, following the WeCom deployment guide's shape. - Add "在微信中验证": send /status, and document both real outcomes — the authorized reply and the refusal that carries the user_id needed for pairing. - Document the message dedupe (`user_id:message_id`) so re-testing uses a new message. - Correct the fabricated /opt/codewhale/weixin-bot-bridge path and env filename, and note that the bridge reads process.env rather than a .env file. Implement the definition: - Default WEIXIN_THREAD_MAP_PATH inside WEIXIN_STATE_DIR. Setting only WEIXIN_STATE_DIR left the thread map at /var/lib/codewhale-weixin-bot-bridge, so every incoming message failed on write; the exception was swallowed by the getUpdates catch and the message was silently dropped. - Create the state dir in saveSyncBuf, matching saveAccount. The poll cursor can be the first persisted write on a fresh install. - Probe thread-map writability at startup and exit with a readable message instead of failing later inside the message loop. - Log the resolved thread map path next to the state dir. Make the login QR scannable in place: - Add src/qr.mjs, a zero-dependency byte-mode QR encoder (versions 1-10, ECC level L) rendered with half-block glyphs, matching the Rust side's Dense1x2 renderer (crates/tui/src/runtime_api.rs). The bridge keeps its no-npm-deps property. - Keep printing the URL below the QR so a terminal that mangles half-block glyphs still has a path through. Single-terminal launcher: - scripts/dev-up.sh plus `npm run bridge` start the runtime and the bridge in one terminal, generate a shared CODEWHALE_RUNTIME_TOKEN, wait for /health, and clean up the runtime on Ctrl-C. Ignore bridge state and re-include bridge launchers: - integrations/*-bridge/.state/ holds the live bot token (account.json) and the poll cursor. - `*.sh` was hiding integrations/*/scripts/, which would have shipped a package.json pointing at a file that never landed. Verification: - npm --prefix integrations/weixin-bridge run check - npm --prefix integrations/weixin-bridge test (12 passed) - npm run bridge logged in by QR and replied to /status from a real WeChat account; the thread map and poll cursor landed under WEIXIN_STATE_DIR. - Terminal QR output decoded back to the exact login URL with OpenCV (9/9 payloads, including UTF-8). Co-Authored-By: DeepSeek-V41-Flash <service@deepseek.com>
|
Thanks @VincentCorleone for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // timing column every subsequent column is offset. | ||
| let upward = true; | ||
| for (let col = size - 1; col > 0; col -= 2) { | ||
| if (col <= 6) col -= 1; |
There was a problem hiding this comment.
🔴 Login QR drops data columns
For every payload, placeData shifts each pair after column six and skips required modules. The QR truncates codewords, so login scanning fails.
Learn more
QR data placement traverses two-column stripes from right to left. Only the stripe that reaches timing column 6 needs shifting; later stripes must continue as columns 5/4, 3/2, and 1/0. The <= 6 condition shifts again at column 3, processes columns 2/1, and ends the loop without visiting columns 3 or 0. Codeword bits remain unplaced, so the format metadata describes a matrix whose payload and error-correction bytes are incomplete.
Example: A version-1 matrix reaches col = 6, correctly shifts to 5, and fills columns 5/4. The next iteration reaches 3, but the current guard shifts it to 2; columns 3 and 0 are never filled.
Recommended fix: Shift only when col === 6, matching the QR placement traversal around the fixed timing column.
| if (col <= 6) col -= 1; | |
| if (col === 6) col -= 1; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| export WEIXIN_ALLOW_UNLISTED=true | ||
| npm start |
There was a problem hiding this comment.
🟡 Two-terminal startup requires root
A non-root user running npm start leaves WEIXIN_STATE_DIR under /var/lib. The startup probe gets EACCES and exits before login.
| export WEIXIN_ALLOW_UNLISTED=true | |
| npm start | |
| export WEIXIN_ALLOW_UNLISTED=true | |
| export WEIXIN_STATE_DIR="$PWD/.state" | |
| npm start |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const dir = path.dirname(config.threadMapPath); | ||
| await fs.mkdir(dir, { recursive: true, mode: 0o700 }); | ||
| const probe = path.join(dir, ".write-probe"); | ||
| await fs.writeFile(probe, "", { mode: 0o600 }); | ||
| await fs.rm(probe, { force: true }); |
There was a problem hiding this comment.
🟡 State directory escapes startup probe
When WEIXIN_THREAD_MAP_PATH uses another directory, the probe never checks WEIXIN_STATE_DIR. saveAccount or saveSyncBuf then fails after startup.
Learn more
The bridge writes the thread map to WEIXIN_THREAD_MAP_PATH, but it always writes account credentials and the polling cursor beneath WEIXIN_STATE_DIR. These locations differ whenever the thread-map override is configured. Probing only the thread-map parent therefore cannot establish the documented state-directory writability guarantee.
Example: Set WEIXIN_STATE_DIR=/var/lib/codewhale-weixin-bot-bridge and WEIXIN_THREAD_MAP_PATH=/tmp/weixin/thread-map.json as an unprivileged user. Startup passes the /tmp/weixin probe, but the first QR login fails while saving account.json under /var/lib.
Recommended fix: Probe both path.dirname(config.threadMapPath) and config.stateDir, deduplicating them when equal. Report which directory failed rather than always labeling it the thread-map directory.
Was this helpful? React with 👍 or 👎 to provide feedback.
| lines.push(invert ? line.replace(/[\u2580\u2584\u2588 ]/g, (ch) => | ||
| ch === " " ? "\u2588" : " ") : line); |
There was a problem hiding this comment.
🟡 Inverted QR loses half modules
With invert: true, renderQrToText replaces every half-block with a space. Mixed cells lose one module, producing a malformed QR.
| lines.push(invert ? line.replace(/[\u2580\u2584\u2588 ]/g, (ch) => | |
| ch === " " ? "\u2588" : " ") : line); | |
| lines.push(invert ? line.replace(/[\u2580\u2584\u2588 ]/g, (ch) => | |
| ch === " " ? "\u2588" : ch === "\u2580" ? "\u2584" : ch === "\u2584" ? "\u2580" : " ") : line); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| // The rendered QR is the login credential, so a wrong matrix is worse than no | ||
| // QR at all. These checks pin the structural invariants a scanner relies on: | ||
| // finder patterns, timing patterns, and the format-information copies. |
There was a problem hiding this comment.
🔍 QR tests miss payload decoding
The suite checks QR structure but never decodes the matrix. Data-placement regressions can pass while login scanning fails.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // QR code encoder — byte mode, versions 1-10, error correction level L. | ||
| // | ||
| // Zero-dependency, so the Weixin bridge keeps its no-npm-deps property. The | ||
| // login URL is ASCII (~70 bytes), which fits version 4-5 at ECC L; versions up | ||
| // to 10 are supported for headroom. | ||
| // | ||
| // Rendered the same way the Rust side does it | ||
| // (`qrcode::render::unicode::Dense1x2` in crates/tui/src/runtime_api.rs): | ||
| // two QR rows per text row using half-block glyphs. |
There was a problem hiding this comment.
🔍 Duplicate QR implementation needs review
The new encoder mirrors the existing Rust qrcode path instead of reusing it. This adds a second protocol implementation under the ponytail rule.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export CODEWHALE_RUNTIME_TOKEN | ||
|
|
||
| export CODEWHALE_RUNTIME_URL="$runtime_url" | ||
| export WEIXIN_ALLOW_UNLISTED="${WEIXIN_ALLOW_UNLISTED:-true}" |
There was a problem hiding this comment.
🟨 Bridge defaults to unrestricted chat access
npm run bridge sets WEIXIN_ALLOW_UNLISTED=true on every launch. Any user reaching the bot can control its agent runtime until explicitly restricted.
Was this helpful? React with 👍 or 👎 to provide feedback.
| # Wait for /health before handing over to the bridge, so the first pairing | ||
| # message does not race a runtime that has not bound its port yet. | ||
| for _ in $(seq 1 60); do | ||
| if curl -fsS "$runtime_url/health" >/dev/null 2>&1; then |
There was a problem hiding this comment.
🟨 Unauthenticated health check trusts wrong service
Any successful /health response satisfies startup without verifying the generated token. A different local service can receive subsequent authenticated bridge requests.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
The Weixin bridge could not be used as documented.
integrations/weixin-bridge/README.mdpointed at a non-existent/opt/codewhale/weixin-bot-bridgepath, gave no runtime startup command, and named an env file that nothing reads. Worse, once it did run, the first incoming WeChat message failed withEACCESand was dropped silently: the thread map defaulted to/var/lib/codewhale-weixin-bot-bridgeregardless ofWEIXIN_STATE_DIR, so every message failed on write inside thegetUpdatescatch. A first-time user saw a scannable QR, a successful login, and then no reply and no actionable error.This PR defines a Quick Start in the README (modeled on
integrations/wecom-bridge/DEPLOYMENT.md) and implements the behavior that section describes.Fix the write path and make the failures visible
WEIXIN_THREAD_MAP_PATHinsideWEIXIN_STATE_DIR. Setting onlyWEIXIN_STATE_DIRleft the map at the old absolute path, so every incoming message failed on write and the message was silently dropped.saveSyncBuf, matchingsaveAccount. The poll cursor can be the first persisted write on a fresh install.Make the login QR scannable in place
src/qr.mjs: a zero-dependency byte-mode QR encoder (versions 1-10, ECC level L) rendered with half-block glyphs, matching the Rust side'sDense1x2renderer incrates/tui/src/runtime_api.rs. The bridge keeps its no-npm-deps property.Add the missing local setup path
scripts/dev-up.shplusnpm run bridgestart the runtime and the bridge in one terminal, generate a sharedCODEWHALE_RUNTIME_TOKEN, wait for/health, and clean up the runtime onCtrl-C..gitignore: ignoreintegrations/*-bridge/.state/(it holds the live bot token inaccount.json), and re-includeintegrations/*/scripts/**, which the global*.shrule was hiding — without that exceptionpackage.jsonwould ship pointing at a file that never landed.Simplify the Quick Start and add screen captures
终端发起(two-terminal and single-terminal) and the two WeChat-side steps,微信端扫码接应and微信端验证信道效果.docs/assets/showing the QR login and a real/statusexchange. Both are redacted before publishing: the local username is removed from the logged paths, the one-timeqrcode=login token is masked, and the QR's bottom-right corner is blacked out so the published code can no longer be scanned.首次配对step 3 wording: the bridge returns runtime status when the sender is authorized, and returns the refusal carrying theuser_idonly when they are not allowlisted.Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features --locked(warning-free under the CI allow list)cargo test --workspace --all-features --lockedThe Rust gate boxes above are not applicable to this change and were not run: no Rust source is touched. The diff is confined to
integrations/weixin-bridge/and.gitignore. I did not run them because they would answer nothing about this diff.Commands actually run, all from
integrations/weixin-bridge/:Direct product evidence, not assertions over internals:
npm run bridgecompleted QR login against the real iLink endpoint, and the same bridge then answered/statusfrom a real WeChat account withruntime=ok. That exchange is the second added capture.cv2.QRCodeDetector), 9/9 payloads including UTF-8, so the printed code is machine-readable rather than merely QR-shaped.Thread map directory is not writable: ...instead of dropping messages later.Checklist
Notes on the unchecked boxes:
src/qr.mjsis a new module, but it is not a layer over an existing one. It has no predecessor to name or delete, and the bridge has no prior QR dependency (package.jsonhas nodependenciesfield at all). It is documented as matching the existing Rust-side renderer so the two stay consistent.Co-Authored-By: DeepSeek-V41-Flash <service@deepseek.com>, which is DeepSeek's publishedContact Emailfrom its Terms of Use. It is not a GitHub numeric noreply address, so I left this box unchecked rather than claim credit registration that will not happen. Maintainers may drop the trailer at merge time if they prefer it gone.Notes for the reviewer
.github/APPROVED_CONTRIBUTORS, so the PR gate may hold the workflow runs ataction_required. A/lgtmon this PR should clear it.crates/tui/src/remote_setup/registry.rs, so/setup bridgeandcodewhale remote-setup --bridge weixindo not list it. That is a separate change against the Rust registry and I did not bundle it here.No-Issue: fixes a local setup defect in integrations/weixin-bridge found by running it; no tracking issue exists.