Skip to content

feat(remote): mobile companion Wave 1 — Feishu + WeChat scan-to-connect - #1379

Closed
Astro-Han wants to merge 11 commits into
devfrom
claude/remote-control-wave1
Closed

feat(remote): mobile companion Wave 1 — Feishu + WeChat scan-to-connect#1379
Astro-Han wants to merge 11 commits into
devfrom
claude/remote-control-wave1

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Summary

Wave 1 of the mobile companion: drive this desktop's agent from a phone chat app,
now across Telegram, Feishu/Lark, and WeChat running concurrently, with
best-in-class scan-to-connect pairing and a first-class sidebar surface.

  • Protocol layer (packages/remote-bridge) — a per-platform supervisor that
    isolates failures (one dead token degrades only its channel), a Feishu/Lark
    adapter over the SDK websocket long connection behind a seam, and a WeChat
    adapter over Tencent's official iLink Bot API. All connect locally,
    outbound-only — no relay we operate, no public IP.
  • Desktop integration (packages/desktop-electron)RemoteBridgeRuntime
    generalized from Telegram-only to N platforms under one bridge; a PlatformPairer
    seam keeps it platform-agnostic; the credential store now holds an account array
    (safeStorage, 0o600) with a v1→v2 migration that preserves an existing Telegram
    pairing. Secrets stay main-side and never round-trip over IPC.
  • UI (packages/app) — remote control moves from a Settings tab to a sidebar
    surface (peer of Automations). One event-driven connect dialog serves all three
    platforms: Telegram opens on a token field, Feishu/WeChat open straight on a QR.

Why

Conversation is PawWork's core surface, and the highest-value place to reach it is
the phone. Telegram shipped first; #1188 calls for the chat apps people in this
market actually use — Feishu and WeChat — with a download-open-connect UX that
assumes non-technical users. The hard constraint was staying local (no
self-operated relay); both new platforms clear it (Feishu device-flow registration,
WeChat iLink), so the bridge keeps its no-public-IP shape.

Related Issue

#1188

Human Review Status

Pending

Review Focus

  • Secret boundary: confirm no credential (Telegram token, Feishu app secret,
    WeChat bot token) can cross back to the renderer — confirmPairing carries no
    secret, status is masked, the store file never returns over IPC.
  • Runtime lifecycle: the serialized queue + single App from N accounts, the
    per-channel status mapping from the supervisor, and the sync-abort on stop().
  • WeChat iLink as the local path (reply-only, no proactive push) — see the
    rationale and the three-pass research note in packages/remote-bridge/README.md.
  • Connect dialog event flow (qr → bind → captured) and the cancel/supersede
    guards.

Risk Notes

  • New runtime dependency qrcode (main process only) renders the Feishu launcher
    URL into a scannable QR; isolated to remote-pairers.ts.
  • Credential file format bumps v1→v2 (single object → account array) with an
    in-place migration for existing Telegram users; a non-migratable file degrades
    to "not connected", never a crash.
  • Platform surface: secrets are persisted via Electron safeStorage on macOS and
    Windows (refuses to persist where OS encryption is unavailable).
  • Priority label awaits the priority-triage bot.

How To Verify

Typecheck (tsgo): remote-bridge ok; desktop-electron (-b, builds app+ui) ok; app ok
ESLint (changed app + desktop files): 0 errors
desktop-electron unit (bun test src/main): 336 pass
  - remote-bridge runtime + credential store: 20 pass (multi-account connect,
    per-channel degrade, pairing cancel/supersede, double-confirm, v1 migration)
app unit: layout/sidebar 240 pass; i18n parity + remote placeholders + connect-toast 3 pass
remote-bridge unit (pre-existing, unchanged): 166 pass
Visual / user-path E2E: `bun run snap remote-surface` — 1 passed; reviewed the grid
  (disconnected/connected/degraded page; token/QR/bind/confirm/disconnect dialog)

Screenshots or Recordings

bun run snap remote-surface produces a 9-cell grid covering every state the user
sees (page: disconnected / connected / degraded; dialog: Telegram token + message
bind, Feishu QR + group bind, captured confirm, disconnect confirm). Reviewed
locally; the grid PNG lives under the git-excluded docs/design/preview/screenshots/,
so run the snap target to regenerate it.

Checklist

  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy

Summary by CodeRabbit

  • New Features

    • Added dedicated Remote Control interface accessible from sidebar with support for multiple platforms (Telegram, Feishu, WeChat).
    • Enables simultaneous connections across multiple platforms with per-platform status and controls.
    • Streamlined pairing flows with platform-specific authentication methods (QR codes, tokens, device flows).
  • Bug Fixes & Improvements

    • Moved Remote Control out of settings into a dedicated, full-page surface for better accessibility and discoverability.

Mint a Feishu/Lark "personal agent" via the OAuth device-authorization flow:
the user scans a QR, Feishu hands back the App ID + App Secret directly — no
manual app creation, no public webhook, no relay. Begin always starts on
accounts.feishu.cn (the launcher QR is minted there even for Lark tenants);
polling then detects a Lark tenant via tenant_brand and switches domains.

Pairing primitive only — it mints credentials; the live connection is the
Feishu Platform adapter, mirroring how Telegram splits captureFirstSender from
TelegramPlatform. FormPoster transport seam keeps it unit-tested (9 tests) and
the live endpoint was validated end to end (HTTP 200 begin).

connect-spike.ts is a manual scan-to-connect validation harness, deleted once
the adapter lands.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
Each platform now runs in its own supervision loop, so one channel's failure is
isolated: a dead Feishu token reports "degraded" and retries with exponential
backoff while Telegram keeps serving. Previously a single platform's start()
rejection rejected a shared deferred and tore the whole bridge down — fine for
the Telegram-only runtime, wrong once multiple channels run concurrently.

The fatal-stream path is unchanged: a dead PawWork event stream still tears the
bridge down (nothing works without it); a dead channel does not. A clean
self-stop (an event-driven adapter that registers its callback and returns) ends
that platform's loop without a restart. Readiness is deduped per platform, so a
misbehaving adapter that double-fires onReady cannot stand in for a platform
that has not served yet. run() gains an onStatus callback so the desktop can
render per-channel connection state.

No per-platform AbortController: Platform has no start-time signal (teardown is
stop()) and Wave 1 disconnects a channel by restarting the whole bridge, so an
independent abort would have no consumer; isolation comes from the independent
loops. 5 supervisor tests added; all 145 remote-bridge tests pass.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
…nnection

Feishu/Lark as a bridge Platform, on @larksuiteoapi/node-sdk's websocket long
connection. The SDK is contained behind a FeishuChannel seam (channel.ts): only
channel-lark.ts imports it, so the adapter logic and all tests run without
loading the 26 MB SDK — the same split as Telegram's baseUrl-seamed poller.

The adapter maps the engine's Platform contract onto the channel: connect()
resolves after the first handshake (no backlog to drain — Feishu does not queue
events for an offline long-connection client, unlike Telegram getUpdates), which
fires onReady; messages route through inboundMessage gating; reply() threads
under the triggering message, send() pushes to the chat. Group hygiene: only
messages in the bound chat (allow_chat) that @mention the bot are accepted, the
leading mention is stripped, everything else is dropped silently. A handshake
failure rejects start() so the supervisor degrades and restarts.

captureFeishuChat (pairing.ts) is the second half of pairing, parallel to
Telegram's captureFirstSender: after the device flow mints credentials, connect
and capture the first @mention to learn which group becomes allow_chat.

19 tests (registration + adapter + pairing); typecheck clean.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
…poll

WeChat as a bridge Platform, on Tencent's official iLink Bot API (the WeChat
ClawBot slot, released 2026-03). Like Telegram it is raw HTTP behind a baseURL
seam — getupdates holds ~35s, sendmessage posts a reply — so it needs no SDK, no
public IP, and no relay we operate (traffic goes through Tencent's ilinkai
servers, exactly as Telegram goes through api.telegram.org). Pairing is
scan-to-connect: QR login mints a bot token (login.ts), then captureWeChatSender
learns the paired user from the first inbound message.

The one structural difference is delivery: iLink has no proactive push — every
outbound message must echo the context_token from the inbound message it answers.
That token rides in the reply context and refreshes on each user turn, so replies
(including permission/question prompts) go through while the user is conversing.
It also means no reconstructReplyCtx: a target can't be rebuilt from a remote key
after a restart, so a restored push is logged and skipped. This matches the Wave
1 design (phone drives, agent replies, no auto-push) — the limitation is largely
moot for us. iLink is a 1:1 DM, so channelID and userID are the same sender id.

Verified the path with three independent research passes (official docs +
adversarial + OSS source-read): iLink is official, local, no-self-relay, and
reply-only. 11 tests; full remote-bridge suite green (166).

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
…tivity)

Companion to the adapter contract in src/platforms/README.md. Captures the parts
the code can't express: the routing/concurrency model (platform-scoped remote
keys, single-target no-broadcast, serialized handling), the Wave 1 desktop-silence
decisions (no auto-mirror, no presence, no handoff command) and why, the
supervisor's failure-isolation model, and the source-verified connectivity for
each platform — including the WeChat iLink finding (official, local, no-self-relay,
reply-only) that corrected the earlier relay assumption.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
… store

Generalize the desktop mobile-companion integration from Telegram-only to N
platforms (Telegram, Feishu/Lark, WeChat) running concurrently under one bridge.

- desktop-api contract: RemoteStatus becomes a per-channel list, pairing is an
  event stream (qr -> awaitingBind -> captured / error / cancelled), and the
  RemoteBridge IPC widens to take a platform per call.
- RemoteBridgeRuntime: holds at most one account per platform, builds a single
  App from all of them (the gateway supervises each channel), and maps the
  per-platform supervisor status onto independent channel state — a dead Feishu
  token degrades while Telegram stays connected. Secrets stay main-side; confirm
  approves a captured account with no secret over IPC.
- PlatformPairer seam keeps the runtime platform-agnostic; remote-pairers.ts wires
  the real Telegram / Feishu device-flow / WeChat iLink flows and is the only
  desktop file that loads the Lark SDK, so the runtime test stays SDK-free.
- Credential store persists an account array (safeStorage, 0o600) with a v1->v2
  migration that preserves an existing single Telegram pairing.

Tests: 20 runtime + credential-store tests (multi-account connect, per-channel
degrade, pairing cancel/supersede, double-confirm, v1 migration). remote-bridge
and desktop-electron main/preload typecheck clean; the app-package UI consumers
are replaced in the follow-up commit.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
…onnect

Move the mobile companion out of a Settings tab into a first-class sidebar
surface (peer of Automations), and build the three connect flows on the new
multi-platform contract.

- New /remote surface: per-channel rows (Telegram, Feishu/Lark, WeChat) each with
  a 2px status left-rule (green connected, red degraded), paired identity, and a
  Connect / Disconnect action. Removed the Settings "Remote access" tab.
- One connect dialog for all platforms, driven by the main-process pairing event
  stream (onPairing): Telegram opens on a token field then the message bind; Feishu
  and WeChat open straight on a QR (scan-to-connect) then the group / message bind;
  all converge on a captured-identity confirm. Secrets never round-trip — confirm
  approves with no secret.
- Feishu hands back a launcher URL, so the QR is rendered main-side (Node qrcode)
  and emitted as an image, unifying the renderer to <img> for both QR platforms,
  with a url + code fallback.
- i18n: new remote.* namespace (en + zh), removed the settings.remote.* keys.
- Visual + user-path E2E: remote-surface.snap.ts walks sidebar -> page -> connect
  dialog with a stubbed bridge, snapshotting every state (disconnected / connected
  / degraded, token / QR / bind / confirm / disconnect). Replaces settings-remote.

Verification: app + desktop-electron + remote-bridge typecheck; eslint clean;
app i18n/toast/layout unit tests (240+) and desktop-electron unit tests (336)
green; remote-surface snap reviewed.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
@Astro-Han Astro-Han added enhancement New feature or request app Application behavior and product flows platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions labels Jun 18, 2026
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Replaces the Telegram-only "Remote access" settings page with a new first-class /remote surface supporting Telegram, Feishu/Lark, and WeChat. The backend adds a supervised multi-platform bridge runtime, per-platform credential storage, three platform adapters with their pairing flows, updated IPC handlers, and a new Electron preload surface. The frontend introduces a sidebar button, a per-platform channel list page, and a unified multi-phase connect dialog.

Changes

Multi-Platform Remote Control

Layer / File(s) Summary
IPC contract: multi-platform types and preload bridge
packages/app/src/desktop-api-contract.ts, packages/app/src/desktop-api.ts, packages/desktop-electron/src/preload/index.ts
RemoteStatus restructured to { channels: RemoteChannelStatus[] }, RemotePlatform/RemoteChannelStatus/RemotePairingEvent/RemotePairingStart added, RemoteBridge methods updated to take platform arguments, onPairing subscription added; preload bridge forwards all new arguments over IPC.
Credential store: multi-account array with v2 migration
packages/desktop-electron/src/main/remote-credentials.ts, packages/desktop-electron/src/main/remote-credentials.test.ts
safeStorageCredentialStore migrated from a single RemoteCredentials object (v1) to an encrypted array of RemoteAccount entries (v2) with a platform-specific isAccount guard and a v1→v2 migration path; tests updated throughout.
Bridge runtime: multi-platform channel manager
packages/desktop-electron/src/main/remote-bridge.ts, packages/desktop-electron/src/main/remote-bridge.test.ts
RemoteBridgeRuntime rewritten to manage concurrent platform channels via statusMap and the new PlatformPairer interface; startPairing becomes event-driven; confirmPairing/disconnect are now platform-scoped; comprehensive test suite covers pairing events, cancellation, concurrent platforms, and startIfConfigured.
Gateway supervisor: per-platform restart loops
packages/remote-bridge/src/supervisor.ts, packages/remote-bridge/src/supervisor.test.ts, packages/remote-bridge/src/gateway.ts, packages/remote-bridge/README.md
New supervisePlatforms runs each Platform under an independent restart loop with starting/serving/degraded phases and exponential backoff; App.run wired to the supervisor with a new onStatus callback; old startPlatforms removed.
Feishu platform adapter
packages/remote-bridge/src/platforms/feishu/*
Complete Feishu/Lark stack: OAuth device-flow registration with Feishu→Lark domain switching, FeishuChannel seam and Lark SDK adapter, captureFeishuChat pairing helper, and FeishuPlatform with group-hygiene filtering, mention stripping, reply threading, and remote-key parsing; fully tested.
WeChat platform adapter
packages/remote-bridge/src/platforms/wechat/*
Complete WeChat/iLink stack: WeChatClient with QR login, long-poll getUpdates, sendMessage with contextToken threading, and fatal/transient error classification; startWeChatLogin/pollWeChatLogin device-flow wrappers; WeChatPlatform with backoff polling loop and captureWeChatSender; fully tested.
Production pairers & Electron wiring
packages/desktop-electron/src/main/remote-pairers.ts, packages/desktop-electron/src/main/ipc/remote.ts, packages/desktop-electron/src/main/index.ts, packages/desktop-electron/electron.vite.config.ts, packages/desktop-electron/electron-vite.config.test.ts, packages/desktop-electron/package.json, packages/remote-bridge/package.json, package.json, patches/@larksuiteoapi%2Fnode-sdk@1.67.0.patch
TelegramPairer, FeishuPairer, WeChatPairer implemented; createRemoteBridgeRuntime moved to remote-pairers; IPC handlers updated to broadcast pairing events and forward platform; @larksuiteoapi/node-sdk and qrcode added as dependencies; axios override and Lark SDK patch applied.
App UI: /remote surface, connect dialog, sidebar, and i18n
packages/app/src/pages/remote/*, packages/app/src/pages/layout/..., packages/app/src/pages/settings/settings-shell.tsx, packages/app/src/i18n/en.ts, packages/app/src/i18n/zh.ts, packages/app/src/i18n/remote-placeholders.test.ts, packages/app/src/app.tsx, packages/app/e2e/...
New /remote route renders RemoteSurface with per-platform ChannelRow and DialogDisconnectRemote; DialogConnectRemote drives a phase state machine (token/starting/qr/bind/confirm/error); PlatformMark SVG logos added; sidebar button wired through layout; old RemotePage/remoteAccess tab removed from settings shell; i18n updated for all three platforms; e2e snapshot test updated.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant RemoteSurface
  participant DialogConnectRemote
  participant PreloadBridge as Preload (window.api.remote)
  participant IPC as Electron Main IPC
  participant Runtime as RemoteBridgeRuntime
  participant Pairer as PlatformPairer

  User->>RemoteSurface: click Connect (e.g. Feishu)
  RemoteSurface->>DialogConnectRemote: open(platform="feishu")
  DialogConnectRemote->>PreloadBridge: onPairing(handler)
  DialogConnectRemote->>PreloadBridge: startPairing("feishu", {domain})
  PreloadBridge->>IPC: remote:start-pairing("feishu", start)
  IPC->>Runtime: startPairing("feishu", start)
  Runtime->>Pairer: pair(start, emit, signal)
  Pairer-->>Runtime: emit PairingEvent(qr, image)
  Runtime-->>IPC: onPairing broadcast
  IPC-->>PreloadBridge: remote:pairing event
  PreloadBridge-->>DialogConnectRemote: PairingEvent(qr)
  DialogConnectRemote->>User: show QR code
  Pairer-->>Runtime: emit PairingEvent(awaitingBind)
  Runtime-->>DialogConnectRemote: PairingEvent(awaitingBind)
  DialogConnectRemote->>User: show bind waiting UI
  Pairer-->>Runtime: emit PairingEvent(captured, identity)
  Runtime-->>DialogConnectRemote: PairingEvent(captured)
  DialogConnectRemote->>User: show confirm identity
  User->>DialogConnectRemote: Allow
  DialogConnectRemote->>PreloadBridge: confirmPairing("feishu")
  PreloadBridge->>IPC: remote:confirm-pairing("feishu")
  IPC->>Runtime: confirmPairing("feishu")
  Runtime->>Runtime: save accounts, startBridge
  Runtime-->>IPC: onStatus({channels:[{state:"connected"}]})
  IPC-->>PreloadBridge: remote:status broadcast
  PreloadBridge-->>RemoteSurface: onStatus update
  RemoteSurface->>User: show Connected + success toast
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • Astro-Han/pawwork#951: Previously migrated the settings shell tab system and scaffolded settings/remote.tsx, which this PR removes and replaces with the new /remote top-level surface.
  • Astro-Han/pawwork#1336: Modified the core remote-bridge gateway and event-handling logic that this PR extends with the supervisor, multi-platform status callbacks, and new platform adapters.
  • Astro-Han/pawwork#1339: Introduced connectToastAction and the Telegram-based DialogConnectRemote, both of which this PR refactors to the multi-platform event-driven model.

Poem

🐇 Three platforms now answer the call,
QR codes bloom on each channel wall,
Feishu scans, WeChat taps in,
Telegram joins with a grin—
Remote control for one and all! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately captures the main feature: Wave 1 mobile companion with multi-platform (Feishu, WeChat) scan-to-connect pairing alongside existing Telegram support.
Description check ✅ Passed Description is comprehensive and well-structured, covering Summary, Why, Related Issue, Review Focus, Risk Notes, and verification steps. All required sections are present and sufficiently detailed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/remote-control-wave1

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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

❤️ Share

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

@github-actions github-actions Bot added ci Continuous integration / GitHub Actions ui Design system and user interface harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority labels Jun 18, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested priority: P2 (includes user-path files (packages/app/src/app.tsx, packages/app/src/components/dialog-connect-remote.tsx, packages/app/src/desktop-api-contract.ts, packages/app/src/desktop-api.ts, packages/app/src/i18n/en.ts, packages/app/src/i18n/remote-placeholders.test.ts, packages/app/src/i18n/zh.ts, packages/app/src/pages/layout.tsx, packages/app/src/pages/layout/pawwork-sidebar-top.tsx, packages/app/src/pages/layout/pawwork-sidebar.tsx, packages/app/src/pages/layout/surface-routes.ts, packages/app/src/pages/remote/connect-toast.test.ts, packages/app/src/pages/remote/connect-toast.ts, packages/app/src/pages/remote/platform-marks.tsx, packages/app/src/pages/remote/remote-connect-dialog.tsx, packages/app/src/pages/remote/remote-route.tsx, packages/app/src/pages/remote/remote-surface.tsx, packages/app/src/pages/settings/remote-connect-toast.test.ts, packages/app/src/pages/settings/remote.tsx, packages/app/src/pages/settings/settings-shell.tsx, packages/desktop-electron/src/main/index.ts, packages/desktop-electron/src/main/ipc/remote.ts, packages/desktop-electron/src/main/remote-bridge.test.ts, packages/desktop-electron/src/main/remote-bridge.ts, packages/desktop-electron/src/main/remote-credentials.test.ts, packages/desktop-electron/src/main/remote-credentials.ts, packages/desktop-electron/src/main/remote-pairers.ts, packages/desktop-electron/src/preload/index.ts)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

The Feishu channel bundled into the Electron main pulls in
@larksuiteoapi/node-sdk, which broke two checks:

- smoke-macos-arm64: bundling the SDK left bare require("protobufjs/
  minimal") calls the runtime could not resolve (Cannot find module).
  Externalize the SDK (electron.vite.config externalizeDeps.include)
  and declare it a direct desktop-electron dependency so it loads whole
  from node_modules like node-pty, with its deep deps resolving beside
  it.
- dev-dep-audit: the SDK transitively pulled axios <1.15.2 (11 high
  advisories). Pin axios 1.18.0 via root overrides (same pattern as the
  existing undici/ws pins).

Verified: `bun audit --audit-level=high` clean; desktop build emits a
clean require("@larksuiteoapi/node-sdk") with no orphan protobufjs
require; runtime-import-guard passes; the SDK and protobufjs/minimal
both resolve and load from out/main.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
Remote control moved out of settings into its own top-level surface, so
the settings shell now has 6 tabs. Update settings-shell.spec.ts to stop
clicking the deleted "Remote access" tab (which failed e2e-artifacts);
the surface itself is covered by remote-surface.snap.ts.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
@Astro-Han

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Pinning axios to 1.18.0 (for the audit) made it violate the SDK's own
declared `axios: ~1.13.3` range. electron-builder's bun dependency
collector runs semver.satisfies(installed, declaredRange) per package and
fails packaging with "production dependency axios not found" because
1.18.0 does not satisfy ~1.13.3 — yet no axios version satisfies both
~1.13.3 and the audit floor (>=1.15.2).

Patch the SDK's declared range to ^1.13.3 so the overridden 1.18.0
satisfies it on disk; the override still drives resolution (and the
audit). Verified: electron-builder --mac dir packaging now completes
through dependency traversal and signing.

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy
The externalize config now lists @larksuiteoapi/node-sdk alongside
node-pty, so the exact-match assertion in electron-vite.config.test.ts
needed updating (this was the unit-desktop / unit-windows-desktop
failure).

Claude-Session: https://claude.ai/code/session_018EtNU58zMFfnFsAtSKaQoy

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (1)
packages/remote-bridge/src/platforms/feishu/pairing.test.ts (1)

54-61: ⚡ Quick win

Add a regression test for abort-before-connect completion.

Current abort coverage aborts after allowConnect(). Add a case that aborts before releasing the connect gate so cancellation behavior is protected when connect is still pending.

Proposed test addition
 test("returns null when aborted before a message arrives", async () => {
   const channel = new FakeChannel()
   const controller = new AbortController()
   const pairing = captureFeishuChat(channel, controller.signal)
   channel.allowConnect()
   controller.abort()
   expect(await pairing).toBeNull()
 })
+
+test("returns null when aborted while connect is still pending", async () => {
+  const channel = new FakeChannel()
+  const controller = new AbortController()
+  const pairing = captureFeishuChat(channel, controller.signal)
+  controller.abort()
+  expect(await pairing).toBeNull()
+})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/remote-bridge/src/platforms/feishu/pairing.test.ts` around lines 54
- 61, Add a new test case alongside the existing test to cover the scenario
where the AbortController is aborted before the connection is allowed to
complete. In this new test, call controller.abort() before calling
channel.allowConnect() on the FakeChannel instance, ensuring that the
captureFeishuChat function properly handles cancellation when the connect
operation is still pending. Both test cases (the existing one that aborts after
allowConnect and the new one that aborts before allowConnect) should verify that
the pairing returns null in both scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/app/src/pages/remote/remote-connect-dialog.tsx`:
- Around line 68-69: The startPairing method is being called without rejection
handling at multiple locations (around the areas with startPairing calls), which
causes unhandled promise rejections and leaves the UI stuck in starting or bind
states instead of transitioning to error. Add proper error handling to each
startPairing call by attaching a catch handler or wrapping in try-catch to
ensure that if the IPC call rejects, the state is properly transitioned to error
rather than remaining in an incomplete state. Make sure all instances of
startPairing (including the ones on lines 68, 83, and 109-112 mentioned in the
comment) have rejection handling that allows the dialog to fail gracefully.

In `@packages/app/src/pages/remote/remote-surface.tsx`:
- Around line 236-245: The handleDisconnect function has a try/finally block but
is missing a catch block to handle errors from the disconnect IPC call. If the
window.api?.remote?.disconnect(props.platform) call fails, the rejection will
bubble up unhandled, leaving the user with no feedback and potentially the busy
state mismanaged. Add a catch block after the try block in handleDisconnect that
catches any error from the disconnect call, logs or displays an error message to
the user (such as via a dialog or toast notification), and ensures the error is
properly handled before the finally block executes to reset the busy state.

In `@packages/desktop-electron/src/main/remote-bridge.ts`:
- Around line 204-207: The issue is that this.accounts is being updated in
memory before the credentials.save() call, which means if the save operation
fails, the runtime state will be out of sync with the persisted state. To fix
this, reorder the operations so that the filtered account list is created as a
temporary variable, saved to disk via this.deps.credentials.save() first, and
only after that promise resolves successfully, assign the result to
this.accounts. Then proceed with calling this.startBridge(). This ensures the
in-memory state only gets updated after the disk write succeeds.

In `@packages/desktop-electron/src/main/remote-credentials.ts`:
- Around line 102-116: The isAccount function validates required credential
fields but fails to validate optional display name fields like userName and
chatName. If these optional fields are present but are not strings, the
identity() function later assumes they are strings and emits malformed data over
IPC. Add validation guards in each case statement (telegram, feishu, wechat) to
ensure that whenever userName or chatName fields are present, they must be of
type string. This should be done by adding additional typeof checks that
validate these optional fields exist as strings if present, preventing
non-string values from being accepted by isAccount and subsequently mishandled
by identity().

In `@packages/remote-bridge/README.md`:
- Around line 17-21: The fenced code block containing the ASCII diagram starting
with the line about phone chat app and Platform adapter is missing a language
identifier, which causes the MD040 linting rule to fail. Add the language
identifier "text" to the opening fence of this code block to make it ```text
instead of just ```, which will properly label the diagram block and satisfy the
documentation lint requirements.

In `@packages/remote-bridge/src/platforms/feishu/channel.ts`:
- Around line 57-59: The stripLeadingMentions function's regex pattern requires
whitespace after each mention, so mention-only messages like "`@PawWork`" with no
trailing whitespace are not stripped. Modify the regex pattern in the replace
call to make the trailing whitespace optional (change \s+ to \s*) so that
mentions at the end of the content string are properly handled and stripped,
preventing downstream logic from treating pure mentions as real command content.

In `@packages/remote-bridge/src/platforms/feishu/pairing.ts`:
- Around line 21-31: The captureFeishuChat function waits for the
channel.connect() call to complete before returning the captured promise, which
means if connect() hangs, the abort signal cannot short-circuit the pending
connection and the function will stall. Fix this by racing the channel.connect()
promise against the captured promise using Promise.race, so that when the abort
signal fires and resolves the captured promise, the function returns immediately
without waiting for connect() to finish. This ensures the abort signal can
interrupt a hanging connect() call.

In `@packages/remote-bridge/src/platforms/feishu/platform.ts`:
- Around line 95-107: There is a race condition where onReady() can fire after
stop() has been requested while connect() is still pending. To fix this, add a
flag property (such as this.isStopped) that is set to true when stopResolve is
invoked, then check this flag immediately before calling onReady() to prevent it
from executing if a stop request has already been made. This ensures the ready
callback does not fire after shutdown has been initiated.

In `@packages/remote-bridge/src/platforms/wechat/client.ts`:
- Around line 175-181: The parse method in the WeChat client silently catches
JSON parsing failures and coerces them to an empty object {}, which masks API
errors when a 2xx response contains malformed JSON. Remove the `.catch(() =>
({}))` exception handler on the `res.json()` call so that JSON parsing failures
properly surface as errors instead of being silently converted to successful
empty responses. This ensures that malformed JSON responses are treated as
failure cases regardless of the HTTP status code.

In `@packages/remote-bridge/src/supervisor.ts`:
- Around line 84-90: The onAbort(signal) call inside the Promise.race block is
being recreated on every retry loop iteration, causing new listeners to
accumulate without being cleaned up. Move the onAbort(signal) expression outside
of the retry loop so that the same promise is reused across all iterations,
preventing listener buildup. This way, the onAbort promise is created once and
shared in the Promise.race call within each loop pass, rather than creating a
fresh listener each time.
- Around line 80-88: The call to platform.start(handler, ready) at line 80 can
throw synchronously, and such synchronous errors will not be caught by the
Promise.race error handler (which only handles promise rejections), causing them
to break the isolation contract where each platform should fail independently.
Wrap the platform.start(handler, ready) call in a way that converts any
synchronous throws into promise rejections. Use Promise.resolve().then(() =>
platform.start(handler, ready)) or place the call inside a try-catch block that
converts caught errors into a rejected promise that can be properly handled by
the existing Promise.race logic.

---

Nitpick comments:
In `@packages/remote-bridge/src/platforms/feishu/pairing.test.ts`:
- Around line 54-61: Add a new test case alongside the existing test to cover
the scenario where the AbortController is aborted before the connection is
allowed to complete. In this new test, call controller.abort() before calling
channel.allowConnect() on the FakeChannel instance, ensuring that the
captureFeishuChat function properly handles cancellation when the connect
operation is still pending. Both test cases (the existing one that aborts after
allowConnect and the new one that aborts before allowConnect) should verify that
the pairing returns null in both scenarios.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b77f5a8d-a6e0-45ad-99d8-c73a7868b8bb

📥 Commits

Reviewing files that changed from the base of the PR and between 00738da and accbe29.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (55)
  • package.json
  • packages/app/e2e/settings/settings-shell.spec.ts
  • packages/app/e2e/snap/remote-surface.snap.ts
  • packages/app/e2e/snap/settings-remote.snap.ts
  • packages/app/src/app.tsx
  • packages/app/src/components/dialog-connect-remote.tsx
  • packages/app/src/desktop-api-contract.ts
  • packages/app/src/desktop-api.ts
  • packages/app/src/i18n/en.ts
  • packages/app/src/i18n/remote-placeholders.test.ts
  • packages/app/src/i18n/zh.ts
  • packages/app/src/pages/layout.tsx
  • packages/app/src/pages/layout/pawwork-sidebar-top.tsx
  • packages/app/src/pages/layout/pawwork-sidebar.tsx
  • packages/app/src/pages/layout/surface-routes.ts
  • packages/app/src/pages/remote/connect-toast.test.ts
  • packages/app/src/pages/remote/connect-toast.ts
  • packages/app/src/pages/remote/platform-marks.tsx
  • packages/app/src/pages/remote/remote-connect-dialog.tsx
  • packages/app/src/pages/remote/remote-route.tsx
  • packages/app/src/pages/remote/remote-surface.tsx
  • packages/app/src/pages/settings/remote-connect-toast.test.ts
  • packages/app/src/pages/settings/remote.tsx
  • packages/app/src/pages/settings/settings-shell.tsx
  • packages/desktop-electron/electron-vite.config.test.ts
  • packages/desktop-electron/electron.vite.config.ts
  • packages/desktop-electron/package.json
  • packages/desktop-electron/src/main/index.ts
  • packages/desktop-electron/src/main/ipc/remote.ts
  • packages/desktop-electron/src/main/remote-bridge.test.ts
  • packages/desktop-electron/src/main/remote-bridge.ts
  • packages/desktop-electron/src/main/remote-credentials.test.ts
  • packages/desktop-electron/src/main/remote-credentials.ts
  • packages/desktop-electron/src/main/remote-pairers.ts
  • packages/desktop-electron/src/preload/index.ts
  • packages/remote-bridge/README.md
  • packages/remote-bridge/package.json
  • packages/remote-bridge/src/gateway.ts
  • packages/remote-bridge/src/platforms/feishu/channel-lark.ts
  • packages/remote-bridge/src/platforms/feishu/channel.ts
  • packages/remote-bridge/src/platforms/feishu/connect-spike.ts
  • packages/remote-bridge/src/platforms/feishu/pairing.test.ts
  • packages/remote-bridge/src/platforms/feishu/pairing.ts
  • packages/remote-bridge/src/platforms/feishu/platform.test.ts
  • packages/remote-bridge/src/platforms/feishu/platform.ts
  • packages/remote-bridge/src/platforms/feishu/registration.test.ts
  • packages/remote-bridge/src/platforms/feishu/registration.ts
  • packages/remote-bridge/src/platforms/wechat/client.test.ts
  • packages/remote-bridge/src/platforms/wechat/client.ts
  • packages/remote-bridge/src/platforms/wechat/login.ts
  • packages/remote-bridge/src/platforms/wechat/platform.test.ts
  • packages/remote-bridge/src/platforms/wechat/platform.ts
  • packages/remote-bridge/src/supervisor.test.ts
  • packages/remote-bridge/src/supervisor.ts
  • patches/@larksuiteoapi%2Fnode-sdk@1.67.0.patch
💤 Files with no reviewable changes (4)
  • packages/app/src/pages/settings/remote-connect-toast.test.ts
  • packages/app/e2e/snap/settings-remote.snap.ts
  • packages/app/src/components/dialog-connect-remote.tsx
  • packages/app/src/pages/settings/remote.tsx

Comment on lines +68 to +69
if (platform !== "telegram") void api.startPairing(platform)
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle startPairing rejections so the dialog can fail gracefully.

Line 68, Line 83, and Line 111 fire startPairing without rejection handling. If the IPC call rejects, this creates unhandled rejections and can strand the UI in starting/bind instead of transitioning to error.

Proposed fix
@@
   onMount(() => {
     const api = remote()
     if (!api) return
     onCleanup(api.onPairing(handlePairing))
     // QR platforms have nothing to type — kick the flow off immediately.
-    if (platform !== "telegram") void api.startPairing(platform)
+    if (platform !== "telegram") {
+      void api.startPairing(platform).catch((err) => {
+        if (alive.value) setStore({ phase: "error", error: errorMessage(err), busy: false })
+      })
+    }
   })
@@
   function submitToken(event?: Event) {
@@
-    void api.startPairing("telegram", { token })
+    void api.startPairing("telegram", { token }).catch((err) => {
+      if (alive.value) setStore({ phase: "error", error: errorMessage(err), busy: false })
+    })
   }
@@
   function retry() {
     const api = remote()
+    if (!api) return
     setStore({ error: undefined, captured: undefined, qr: undefined })
@@
-    void api?.startPairing(platform)
+    void api.startPairing(platform).catch((err) => {
+      if (alive.value) setStore({ phase: "error", error: errorMessage(err), busy: false })
+    })
   }

Also applies to: 83-84, 109-112

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app/src/pages/remote/remote-connect-dialog.tsx` around lines 68 -
69, The startPairing method is being called without rejection handling at
multiple locations (around the areas with startPairing calls), which causes
unhandled promise rejections and leaves the UI stuck in starting or bind states
instead of transitioning to error. Add proper error handling to each
startPairing call by attaching a catch handler or wrapping in try-catch to
ensure that if the IPC call rejects, the state is properly transitioned to error
rather than remaining in an incomplete state. Make sure all instances of
startPairing (including the ones on lines 68, 83, and 109-112 mentioned in the
comment) have rejection handling that allows the dialog to fail gracefully.

Comment on lines +236 to +245
const handleDisconnect = async () => {
if (busy()) return
setBusy(true)
try {
await window.api?.remote?.disconnect(props.platform)
dialog.close()
} finally {
setBusy(false)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle disconnect IPC failures explicitly.

At Line [240], a rejected disconnect call bubbles out of the click handler, which can leave an unhandled rejection and no user feedback.

Proposed fix
   const handleDisconnect = async () => {
     if (busy()) return
     setBusy(true)
     try {
       await window.api?.remote?.disconnect(props.platform)
       dialog.close()
+    } catch {
+      showToast({
+        variant: "error",
+        title: language.t("common.requestFailed"),
+      })
     } finally {
       setBusy(false)
     }
   }
📝 Committable suggestion

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

Suggested change
const handleDisconnect = async () => {
if (busy()) return
setBusy(true)
try {
await window.api?.remote?.disconnect(props.platform)
dialog.close()
} finally {
setBusy(false)
}
}
const handleDisconnect = async () => {
if (busy()) return
setBusy(true)
try {
await window.api?.remote?.disconnect(props.platform)
dialog.close()
} catch {
showToast({
variant: "error",
title: language.t("common.requestFailed"),
})
} finally {
setBusy(false)
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app/src/pages/remote/remote-surface.tsx` around lines 236 - 245, The
handleDisconnect function has a try/finally block but is missing a catch block
to handle errors from the disconnect IPC call. If the
window.api?.remote?.disconnect(props.platform) call fails, the rejection will
bubble up unhandled, leaving the user with no feedback and potentially the busy
state mismanaged. Add a catch block after the try block in handleDisconnect that
catches any error from the disconnect call, logs or displays an error message to
the user (such as via a dialog or toast notification), and ensures the error is
properly handled before the finally block executes to reset the busy state.

Comment on lines 204 to +207
await this.enqueue(async () => {
await this.stopBridge()
const creds: RemoteCredentials = { ...pending }
this.deps.credentials.save(creds)
await this.startBridge(creds)
this.accounts = [...this.accounts.filter((account) => account.platform !== platform), pending]
this.deps.credentials.save(this.accounts)
await this.startBridge()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Commit account changes only after the credential write succeeds.

Both paths assign this.accounts before credentials.save(...). If the safeStorage write or chmod step throws, the runtime can keep an unsaved account list while the persisted credentials and live bridge still reflect the old state.

Suggested persistence ordering fix
     this.pending = null
     await this.enqueue(async () => {
-      this.accounts = [...this.accounts.filter((account) => account.platform !== platform), pending]
-      this.deps.credentials.save(this.accounts)
+      const nextAccounts = [...this.accounts.filter((account) => account.platform !== platform), pending]
+      this.deps.credentials.save(nextAccounts)
+      this.accounts = nextAccounts
       await this.startBridge()
     })
   }
@@
     if (this.pending?.platform === platform) this.cancelPairing()
     await this.enqueue(async () => {
-      this.accounts = this.accounts.filter((account) => account.platform !== platform)
-      this.deps.credentials.save(this.accounts)
+      const nextAccounts = this.accounts.filter((account) => account.platform !== platform)
+      this.deps.credentials.save(nextAccounts)
+      this.accounts = nextAccounts
       this.statusMap.delete(platform)
       this.emitStatus()

Also applies to: 215-216

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/desktop-electron/src/main/remote-bridge.ts` around lines 204 - 207,
The issue is that this.accounts is being updated in memory before the
credentials.save() call, which means if the save operation fails, the runtime
state will be out of sync with the persisted state. To fix this, reorder the
operations so that the filtered account list is created as a temporary variable,
saved to disk via this.deps.credentials.save() first, and only after that
promise resolves successfully, assign the result to this.accounts. Then proceed
with calling this.startBridge(). This ensures the in-memory state only gets
updated after the disk write succeeds.

Comment on lines +102 to +116
case "telegram":
return typeof account.token === "string" && typeof account.allowFrom === "string"
case "feishu":
return (
typeof account.appId === "string" &&
typeof account.appSecret === "string" &&
typeof account.allowChat === "string" &&
(account.domain === "feishu" || account.domain === "lark")
)
case "wechat":
return (
typeof account.botToken === "string" &&
typeof account.baseURL === "string" &&
typeof account.allowFrom === "string"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate optional display names before narrowing the account.

isAccount accepts records where userName or chatName is present but not a string; identity() later trusts those fields as strings and can emit malformed RemoteChannelStatus.identity.name over IPC.

Suggested guard tightening
     case "telegram":
-      return typeof account.token === "string" && typeof account.allowFrom === "string"
+      return (
+        typeof account.token === "string" &&
+        typeof account.allowFrom === "string" &&
+        (account.userName === undefined || typeof account.userName === "string")
+      )
     case "feishu":
       return (
         typeof account.appId === "string" &&
         typeof account.appSecret === "string" &&
         typeof account.allowChat === "string" &&
-        (account.domain === "feishu" || account.domain === "lark")
+        (account.domain === "feishu" || account.domain === "lark") &&
+        (account.chatName === undefined || typeof account.chatName === "string")
       )
     case "wechat":
       return (
         typeof account.botToken === "string" &&
         typeof account.baseURL === "string" &&
-        typeof account.allowFrom === "string"
+        typeof account.allowFrom === "string" &&
+        (account.userName === undefined || typeof account.userName === "string")
       )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/desktop-electron/src/main/remote-credentials.ts` around lines 102 -
116, The isAccount function validates required credential fields but fails to
validate optional display name fields like userName and chatName. If these
optional fields are present but are not strings, the identity() function later
assumes they are strings and emits malformed data over IPC. Add validation
guards in each case statement (telegram, feishu, wechat) to ensure that whenever
userName or chatName fields are present, they must be of type string. This
should be done by adding additional typeof checks that validate these optional
fields exist as strings if present, preventing non-string values from being
accepted by isAccount and subsequently mishandled by identity().

Comment on lines +17 to +21
```
phone chat app ──outbound──▶ Platform adapter ─┐
├─▶ Engine ──HTTP+SSE──▶ local PawWork server
phone chat app ──outbound──▶ Platform adapter ─┘
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to the fenced diagram block.

The fence at Line 17 is unlabeled (MD040), which can fail docs lint in CI.

Proposed fix
-```
+```text
 phone chat app ──outbound──▶ Platform adapter ─┐
                                                ├─▶ Engine ──HTTP+SSE──▶ local PawWork server
 phone chat app ──outbound──▶ Platform adapter ─┘
</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 17-17: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @packages/remote-bridge/README.md around lines 17 - 21, The fenced code block
containing the ASCII diagram starting with the line about phone chat app and
Platform adapter is missing a language identifier, which causes the MD040
linting rule to fail. Add the language identifier "text" to the opening fence of
this code block to make it text instead of just , which will properly
label the diagram block and satisfy the documentation lint requirements.


</details>

<!-- fingerprinting:phantom:poseidon:hawk -->

<!-- cr-comment:v1:dd494346e7d36ca50cd82a91 -->

_Source: Linters/SAST tools_

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +21 to +31
export async function captureFeishuChat(channel: FeishuChannel, signal: AbortSignal): Promise<FeishuPairedChat | null> {
if (signal.aborted) return null
const captured = new Promise<FeishuPairedChat | null>((resolve) => {
channel.onMessage((msg) => {
if (msg.chatType === "group" && msg.mentionedBot) resolve({ chatId: msg.chatId })
})
signal.addEventListener("abort", () => resolve(null), { once: true })
})
await channel.connect()
if (signal.aborted) return null
return captured

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Abort does not short-circuit a pending connect() call.

Lines 29-31 wait for connect() before returning the abort-resolving promise. If connect hangs, cancellation cannot resolve promptly and pairing can stall after dialog close.

Proposed fix
 export async function captureFeishuChat(channel: FeishuChannel, signal: AbortSignal): Promise<FeishuPairedChat | null> {
   if (signal.aborted) return null
+  const aborted = new Promise<null>((resolve) => {
+    signal.addEventListener("abort", () => resolve(null), { once: true })
+  })
   const captured = new Promise<FeishuPairedChat | null>((resolve) => {
     channel.onMessage((msg) => {
       if (msg.chatType === "group" && msg.mentionedBot) resolve({ chatId: msg.chatId })
     })
-    signal.addEventListener("abort", () => resolve(null), { once: true })
+    signal.addEventListener("abort", () => resolve(null), { once: true })
   })
-  await channel.connect()
+  await Promise.race([channel.connect(), aborted])
   if (signal.aborted) return null
   return captured
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/remote-bridge/src/platforms/feishu/pairing.ts` around lines 21 - 31,
The captureFeishuChat function waits for the channel.connect() call to complete
before returning the captured promise, which means if connect() hangs, the abort
signal cannot short-circuit the pending connection and the function will stall.
Fix this by racing the channel.connect() promise against the captured promise
using Promise.race, so that when the abort signal fires and resolves the
captured promise, the function returns immediately without waiting for connect()
to finish. This ensures the abort signal can interrupt a hanging connect() call.

Comment on lines +95 to +107
const stopped = new Promise<void>((resolve) => {
this.stopResolve = resolve
})
try {
// connect() resolves after the first handshake and rejects on a bad
// credential; the supervisor turns a rejection into degraded + backoff.
// There is no backlog to drain, so a clean connect means we are serving.
await channel.connect()
onReady?.()
// The SDK keeps the connection alive (auto-reconnect) in the background;
// hold start() open until stop() so the supervisor treats us as serving.
await stopped
} finally {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Prevent onReady from firing after a stop request during connect.

There’s a lifecycle race between Line 102 (connect) and Line 134 (stopResolve). If stop() happens while connect() is pending, onReady at Line 103 can still fire after shutdown was requested, and the channel may remain open if disconnect happened too early.

Proposed fix
   async start(handler: MessageHandler, onReady?: () => void): Promise<void> {
     if (this.channel) return
+    let stopRequested = false
     const channel = this.opts.createChannel({
       appId: this.opts.appId,
       appSecret: this.opts.appSecret,
       domain: this.opts.domain,
     })
@@
-    const stopped = new Promise<void>((resolve) => {
-      this.stopResolve = resolve
+    const stopped = new Promise<void>((resolve) => {
+      this.stopResolve = () => {
+        stopRequested = true
+        resolve()
+      }
     })
     try {
@@
       await channel.connect()
+      if (stopRequested) {
+        await channel.disconnect().catch(() => {})
+        return
+      }
       onReady?.()
       // The SDK keeps the connection alive (auto-reconnect) in the background;
       // hold start() open until stop() so the supervisor treats us as serving.
       await stopped

Also applies to: 132-136

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/remote-bridge/src/platforms/feishu/platform.ts` around lines 95 -
107, There is a race condition where onReady() can fire after stop() has been
requested while connect() is still pending. To fix this, add a flag property
(such as this.isStopped) that is set to true when stopResolve is invoked, then
check this flag immediately before calling onReady() to prevent it from
executing if a stop request has already been made. This ensures the ready
callback does not fire after shutdown has been initiated.

Comment on lines +175 to +181
private async parse(path: string, res: Response): Promise<Record<string, unknown>> {
const data = (await res.json().catch(() => ({}))) as Record<string, unknown>
const ret = typeof data.ret === "number" ? data.ret : undefined
if (!res.ok || (ret !== undefined && ret !== 0)) {
throw new WeChatApiError(path.split("?")[0], res.status, ret, str(data, "errmsg") || str(data, "message"))
}
return data

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not treat malformed JSON 2xx responses as successful API calls.

At Lines 176-181, JSON parse failure is coerced to {}. If iLink returns a 2xx non-JSON body, the client silently proceeds as success instead of surfacing a degraded/error state.

💡 Suggested fix
   private async parse(path: string, res: Response): Promise<Record<string, unknown>> {
-    const data = (await res.json().catch(() => ({}))) as Record<string, unknown>
+    const endpoint = path.split("?")[0]
+    const raw = await res.text()
+    let data: Record<string, unknown> = {}
+    if (raw.trim() !== "") {
+      try {
+        data = JSON.parse(raw) as Record<string, unknown>
+      } catch {
+        if (!res.ok) throw new WeChatApiError(endpoint, res.status, undefined, raw)
+        throw new WeChatApiError(endpoint, res.status, undefined, "invalid JSON response")
+      }
+    }
     const ret = typeof data.ret === "number" ? data.ret : undefined
     if (!res.ok || (ret !== undefined && ret !== 0)) {
-      throw new WeChatApiError(path.split("?")[0], res.status, ret, str(data, "errmsg") || str(data, "message"))
+      throw new WeChatApiError(endpoint, res.status, ret, str(data, "errmsg") || str(data, "message"))
     }
     return data
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/remote-bridge/src/platforms/wechat/client.ts` around lines 175 -
181, The parse method in the WeChat client silently catches JSON parsing
failures and coerces them to an empty object {}, which masks API errors when a
2xx response contains malformed JSON. Remove the `.catch(() => ({}))` exception
handler on the `res.json()` call so that JSON parsing failures properly surface
as errors instead of being silently converted to successful empty responses.
This ensures that malformed JSON responses are treated as failure cases
regardless of the HTTP status code.

Comment on lines +80 to +88
const startPromise = platform.start(handler, ready)
// Abort can win the race below, leaving start() in flight; keep its eventual
// rejection from surfacing as an unhandled rejection.
startPromise.catch(() => {})
const outcome = await Promise.race([
startPromise.then(
() => ({ failed: false as const }),
(err) => ({ failed: true as const, error: message(err) }),
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard platform.start(...) against synchronous throws to preserve isolation.

At Line 80, a synchronous throw can bypass the per-platform failure branch and reject the supervision aggregate, which breaks the “one channel fails independently” contract.

💡 Suggested fix
-    const startPromise = platform.start(handler, ready)
+    const startPromise = Promise.resolve().then(() => platform.start(handler, ready))
📝 Committable suggestion

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

Suggested change
const startPromise = platform.start(handler, ready)
// Abort can win the race below, leaving start() in flight; keep its eventual
// rejection from surfacing as an unhandled rejection.
startPromise.catch(() => {})
const outcome = await Promise.race([
startPromise.then(
() => ({ failed: false as const }),
(err) => ({ failed: true as const, error: message(err) }),
),
const startPromise = Promise.resolve().then(() => platform.start(handler, ready))
// Abort can win the race below, leaving start() in flight; keep its eventual
// rejection from surfacing as an unhandled rejection.
startPromise.catch(() => {})
const outcome = await Promise.race([
startPromise.then(
() => ({ failed: false as const }),
(err) => ({ failed: true as const, error: message(err) }),
),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/remote-bridge/src/supervisor.ts` around lines 80 - 88, The call to
platform.start(handler, ready) at line 80 can throw synchronously, and such
synchronous errors will not be caught by the Promise.race error handler (which
only handles promise rejections), causing them to break the isolation contract
where each platform should fail independently. Wrap the platform.start(handler,
ready) call in a way that converts any synchronous throws into promise
rejections. Use Promise.resolve().then(() => platform.start(handler, ready)) or
place the call inside a try-catch block that converts caught errors into a
rejected promise that can be properly handled by the existing Promise.race
logic.

Comment on lines +84 to +90
const outcome = await Promise.race([
startPromise.then(
() => ({ failed: false as const }),
(err) => ({ failed: true as const, error: message(err) }),
),
onAbort(signal).then(() => ({ failed: false as const })),
])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid accumulating abort listeners on every retry iteration.

onAbort(signal) is recreated in each loop pass (Lines 84-90), and each call adds a new listener (Line 102) that remains until abort fires. A flapping platform can build up many listeners over time.

💡 Suggested fix
 async function superviseOne(
   platform: Platform,
   handler: MessageHandler,
   signal: AbortSignal,
   options: SuperviseOptions,
 ): Promise<void> {
@@
+  const aborted = onAbort(signal).then(() => ({ failed: false as const }))
   while (!signal.aborted) {
@@
     const outcome = await Promise.race([
       startPromise.then(
         () => ({ failed: false as const }),
         (err) => ({ failed: true as const, error: message(err) }),
       ),
-      onAbort(signal).then(() => ({ failed: false as const })),
+      aborted,
     ])

Also applies to: 100-103

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/remote-bridge/src/supervisor.ts` around lines 84 - 90, The
onAbort(signal) call inside the Promise.race block is being recreated on every
retry loop iteration, causing new listeners to accumulate without being cleaned
up. Move the onAbort(signal) expression outside of the retry loop so that the
same promise is reused across all iterations, preventing listener buildup. This
way, the onAbort promise is created once and shared in the Promise.race call
within each loop pass, rather than creating a fresh listener each time.

@Astro-Han

Copy link
Copy Markdown
Owner Author

Closing in favor of a flat decomposition.

This PR welded four concerns into one +3858/-1089 diff: the platform-agnostic multi-channel runtime, two new protocol adapters (Feishu, WeChat), and the remote-control UI relocation. Per our flat-PR topology, it is being re-landed as independent PRs from dev:

  1. Foundation — platform-agnostic multi-channel bridge: supervisor, the generic runtime + pairer registry, the multi-channel IPC contract, the credentials store. Telegram-only (the one platform already on dev), so it is a self-contained refactor of the existing single-channel path.
  2. WeChat adapter — additive: adapter dir + pairer registration. No new external dependency.
  3. Feishu adapter — additive; the Feishu transport's dependency question (official SDK vs. a thin vendored long-connection client) is reconsidered here, isolated from everything else.
  4. Remote UI — relocate remote control to a sidebar surface with multi-platform display + scan-to-connect.

The branch claude/remote-control-wave1 is retained as the reference implementation until the split lands; nothing here is lost.

https://claude.ai/code/session_013LqJ6qQMaYJrsmz2jafRJd

@Astro-Han Astro-Han closed this Jun 19, 2026
@Astro-Han
Astro-Han deleted the claude/remote-control-wave1 branch August 21, 2026 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows ci Continuous integration / GitHub Actions enhancement New feature or request harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant