fix(remote): deliver weixin generated images#1610
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds end-to-end support for assistant-generated images: decoding from multiple source formats (cached paths, data URLs, raw base64), persisting them with workspace or userData roots, and sending them encrypted via iLink protocol to WeChat. ChangesGenerated Images: Multi-Format Resolution, Storage, and Encrypted Delivery
Sequence DiagramsequenceDiagram
participant AssistantOutput
participant RemoteConversationRunner
participant ContentResolver as resolveGeneratedImageContent
participant AssetRoot as resolveGeneratedImageAssetRoot
participant DiskStorage as File System
participant WeixinClient
participant UploadService as CDN Upload
participant WeChat
AssistantOutput->>RemoteConversationRunner: generatedImages (imgcache://, data URL, base64)
RemoteConversationRunner->>AssetRoot: resolveGeneratedImageAssetRoot(workspace)
AssetRoot-->>RemoteConversationRunner: asset root (workspace or userData)
RemoteConversationRunner->>ContentResolver: resolveGeneratedImageContent(source)
ContentResolver-->>RemoteConversationRunner: { data, mimeType }
RemoteConversationRunner->>DiskStorage: write file with extension from mimeType
RemoteConversationRunner-->>RemoteConversationRunner: persist snapshot with imagePath, mimeType
RemoteConversationRunner->>WeixinClient: sendImageMessage(imagePath, mimeType)
WeixinClient->>WeixinClient: encrypt image bytes with AES key
WeixinClient->>UploadService: POST encrypted bytes, extract x-encrypted-param
UploadService-->>WeixinClient: success, x-encrypted-param header
WeixinClient->>WeChat: send message with encrypt_query_param, aes_key, encrypt_type
WeChat-->>WeixinClient: delivery success
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/main/presenter/remoteControlPresenter/weixinIlink/weixinIlinkClient.ts (3)
612-641: ⚡ Quick winConsider a longer timeout for CDN media uploads.
uploadCdnMediareusesWEIXIN_ILINK_REQUEST_TIMEOUT_MS(15s), which is shared with lightweight JSON RPCs. AI-generated images frequently land in the 1–5 MB range, and on slower mobile/uplink networks a 15s budget for the full POST + CDN ack can be tight. Failures here force the wholesendImageMessageflow to start over (including a freshgetuploadurl).Consider introducing a dedicated upload timeout (e.g. 60s) or making it configurable.
♻️ Proposed change — dedicated media upload timeout
const WEIXIN_ILINK_REQUEST_TIMEOUT_MS = 15_000 const WEIXIN_ILINK_LONG_POLL_TIMEOUT_MS = 35_000 +const WEIXIN_ILINK_MEDIA_UPLOAD_TIMEOUT_MS = 60_000 const WEIXIN_ILINK_CHANNEL_VERSION = '1.0.0'- return await withTimeout(WEIXIN_ILINK_REQUEST_TIMEOUT_MS, async (signal) => { + return await withTimeout(WEIXIN_ILINK_MEDIA_UPLOAD_TIMEOUT_MS, async (signal) => {🤖 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 `@src/main/presenter/remoteControlPresenter/weixinIlink/weixinIlinkClient.ts` around lines 612 - 641, uploadCdnMedia uses the generic WEIXIN_ILINK_REQUEST_TIMEOUT_MS (15s) which is too short for multi-MB CDN POSTs; introduce a dedicated, longer timeout (e.g. WEIXIN_ILINK_CDN_UPLOAD_TIMEOUT_MS defaulting to ~60000ms) or make it configurable, and replace the call to withTimeout(...) inside uploadCdnMedia to use this new timeout constant/setting so large media uploads get a higher deadline without affecting lightweight JSON RPCs.
625-625: 💤 Low valueAvoid unnecessary buffer copy in CDN upload body.
Uint8Array.from(params.content)allocates and copies the encrypted bytes again.Bufferalready extendsUint8Array, soparams.contentcan be passed directly tofetch. For multi-MB images this doubles transient memory for no benefit.♻️ Proposed fix
- body: Uint8Array.from(params.content), + body: params.content,🤖 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 `@src/main/presenter/remoteControlPresenter/weixinIlink/weixinIlinkClient.ts` at line 625, The fetch request is creating an unnecessary copy by using Uint8Array.from(params.content); instead pass params.content directly as the body to avoid duplicating multi-MB buffers—locate the fetch/HTTP upload call in weixinIlinkClient (the code that sets body: Uint8Array.from(params.content)) and replace that expression with params.content (or ensure it's typed as Buffer | Uint8Array) so no extra allocation occurs.
172-179: ⚡ Quick winPreserve original error context when wrapping image-send stage errors.
withImageSendStagere-throws as a plainError, which discards the original error type (e.g.WeixinIlinkApiError) along with itsstatusanderrcodeproperties. Callers can no longer distinguish a CDN HTTP failure from an API errcode, which makes upstream retry/handling and logs less actionable.♻️ Proposed fix — attach `cause` and preserve `WeixinIlinkApiError`
const withImageSendStage = async <T>(stage: string, operation: () => Promise<T>): Promise<T> => { try { return await operation() } catch (error) { const message = error instanceof Error ? error.message : String(error) - throw new Error(`Weixin iLink image ${stage} failed: ${message}`) + const wrappedMessage = `Weixin iLink image ${stage} failed: ${message}` + if (error instanceof WeixinIlinkApiError) { + throw new WeixinIlinkApiError(wrappedMessage, error.status, error.errcode) + } + throw new Error(wrappedMessage, { cause: error }) } }🤖 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 `@src/main/presenter/remoteControlPresenter/weixinIlink/weixinIlinkClient.ts` around lines 172 - 179, The helper withImageSendStage currently catches errors and throws a new Error, losing the original error type and metadata (e.g. WeixinIlinkApiError with status and errcode); change it to rethrow preserving the original error as the cause (or rethrow the original instance when it is already a WeixinIlinkApiError) and when creating a new Error include the original error as the cause property so callers can inspect status/errcode; update withImageSendStage to check instanceof WeixinIlinkApiError and either throw the original error or throw a new Error(`Weixin iLink image ${stage} failed: ${message}`, { cause: error }) to preserve context.src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts (1)
727-737: Consider lifecycle/cleanup for the userData fallback asset root.When no workspace is available, generated images are persisted indefinitely under
userData/remote-assets/<channel>/<hash>/<messageId>/. There is no cleanup path tied to session deletion or message expiry, so over time this directory grows unbounded — especially for users heavily using image-generating agents on bots without aprojectDir. Worth tracking a retention/pruning job (e.g. age-based or session-bound) before this becomes a support issue.🤖 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 `@src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts` around lines 727 - 737, When resolveGeneratedImageAssetRoot falls back to userData (the path built with REMOTE_GENERATED_ASSET_ROOT), ensure those generated-asset directories are lifecycle-managed: add a registry and pruning job for app.getPath('userData')/REMOTE_GENERATED_ASSET_ROOT that records creation timestamps (or ties dirs to session IDs) and periodically removes entries older than a retention TTL or when a session is deleted. Concretely, update resolveGeneratedImageAssetRoot and/or resolveOptionalAssetWorkspace call sites to (a) create session-scoped subdirectories under REMOTE_GENERATED_ASSET_ROOT that include the session.agentId or message id, (b) register the created path with a new cleanup service (e.g., GeneratedAssetCleaner) and (c) wire that cleaner to run at startup and on session end to delete expired dirs; use the constants REMOTE_ASSET_ROOT and REMOTE_GENERATED_ASSET_ROOT to locate targets and ensure the cleaner uses safe delete (verify ownership) and a configurable TTL.
🤖 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.
Nitpick comments:
In
`@src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts`:
- Around line 727-737: When resolveGeneratedImageAssetRoot falls back to
userData (the path built with REMOTE_GENERATED_ASSET_ROOT), ensure those
generated-asset directories are lifecycle-managed: add a registry and pruning
job for app.getPath('userData')/REMOTE_GENERATED_ASSET_ROOT that records
creation timestamps (or ties dirs to session IDs) and periodically removes
entries older than a retention TTL or when a session is deleted. Concretely,
update resolveGeneratedImageAssetRoot and/or resolveOptionalAssetWorkspace call
sites to (a) create session-scoped subdirectories under
REMOTE_GENERATED_ASSET_ROOT that include the session.agentId or message id, (b)
register the created path with a new cleanup service (e.g.,
GeneratedAssetCleaner) and (c) wire that cleaner to run at startup and on
session end to delete expired dirs; use the constants REMOTE_ASSET_ROOT and
REMOTE_GENERATED_ASSET_ROOT to locate targets and ensure the cleaner uses safe
delete (verify ownership) and a configurable TTL.
In `@src/main/presenter/remoteControlPresenter/weixinIlink/weixinIlinkClient.ts`:
- Around line 612-641: uploadCdnMedia uses the generic
WEIXIN_ILINK_REQUEST_TIMEOUT_MS (15s) which is too short for multi-MB CDN POSTs;
introduce a dedicated, longer timeout (e.g. WEIXIN_ILINK_CDN_UPLOAD_TIMEOUT_MS
defaulting to ~60000ms) or make it configurable, and replace the call to
withTimeout(...) inside uploadCdnMedia to use this new timeout constant/setting
so large media uploads get a higher deadline without affecting lightweight JSON
RPCs.
- Line 625: The fetch request is creating an unnecessary copy by using
Uint8Array.from(params.content); instead pass params.content directly as the
body to avoid duplicating multi-MB buffers—locate the fetch/HTTP upload call in
weixinIlinkClient (the code that sets body: Uint8Array.from(params.content)) and
replace that expression with params.content (or ensure it's typed as Buffer |
Uint8Array) so no extra allocation occurs.
- Around line 172-179: The helper withImageSendStage currently catches errors
and throws a new Error, losing the original error type and metadata (e.g.
WeixinIlinkApiError with status and errcode); change it to rethrow preserving
the original error as the cause (or rethrow the original instance when it is
already a WeixinIlinkApiError) and when creating a new Error include the
original error as the cause property so callers can inspect status/errcode;
update withImageSendStage to check instanceof WeixinIlinkApiError and either
throw the original error or throw a new Error(`Weixin iLink image ${stage}
failed: ${message}`, { cause: error }) to preserve context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27a41e5d-ce68-41ea-a854-54e7565d429f
📒 Files selected for processing (4)
src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.tssrc/main/presenter/remoteControlPresenter/weixinIlink/weixinIlinkClient.tstest/main/presenter/remoteControlPresenter/remoteConversationRunner.test.tstest/main/presenter/remoteControlPresenter/weixinIlinkRuntime.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
deliver weixin generated images
Summary by CodeRabbit
Release Notes
New Features
Improvements