feat(harmonyos): add Sunshine clipboard sync#26
Conversation
- add HarmonyOS clipboard text sync end-to-end bridge and settings integration - wire native clipboard callbacks through ArkTS streaming session - degrade image clipboard sync explicitly on current compatible SDK - document current implementation and permission constraints
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough本 PR 在 HarmonyOS 上端到端接入剪贴板同步:协议/帧格式、ClipboardSyncService(本地监听、出入站、回显抑制)、Blob 上传/下载与 HTTP 支持、ArkTS↔Native 桥接(TypedArray 与 TSFN)、流会话回调路由并集成到设置/页面/组件;文本已闭环,图片运行时降级。 变更内容剪贴板同步完整实现
🎯 3 (中等) | ⏱️ ~25 分钟 可能相关的 PRs:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
docs/CLIPBOARD_SYNC_INTEGRATION_HARMONYOS.md (1)
154-165: ⚡ Quick win建议在文档中明确图片同步降级的具体用户体验。
文档说明了图片同步采用"显式保守处理"策略,但对于具体的用户体验描述不够清晰。建议补充:
- 降级提示的具体文案是什么?
- 提示是 Toast 还是其他形式?
- "只提示一次"的逻辑是基于什么(单次会话?应用生命周期?)
📝 建议补充的说明
在第159行"本地或远端一旦进入图片分支,只提示一次不支持"后面添加:
- 本地或远端一旦进入图片分支,只提示一次不支持 + (通过 Toast 提示"图片剪贴板同步暂不支持",每次服务启动仅提示一次) - 不调用未验证图片剪贴板 API🤖 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 `@docs/CLIPBOARD_SYNC_INTEGRATION_HARMONYOS.md` around lines 154 - 165, Update the paragraph that begins "本地或远端一旦进入图片分支,只提示一次不支持" to explicitly document the user-facing UX: include the exact downgrade message text (provide Chinese and optional English), state the UI element used (e.g., system Toast/Snackbar vs modal) and its duration/priority, and define the "只提示一次" scope (e.g., once per app session, once per device until user dismisses, or persisted via a flag in local storage) plus how the flag is cleared; ensure these additions appear immediately after the referenced sentence so readers can find the concrete message, display mechanism, and the precise persistence/clearing logic for the one-time notification.
🤖 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 `@entry/src/main/ets/pages/StreamPage.ets`:
- Around line 1321-1335: The clipboard sync initialization is only in
startStreaming() so after a disconnect and reconnect (reconnectStreaming()) the
ClipboardSyncService isn't restarted; extract the clipboard startup into a
reusable method (e.g., startClipboardSync or initClipboardSync) that builds
ClipboardSyncConfig, creates StreamClipboardObserver, instantiates
ClipboardSyncService and calls start(), then call this new method from
startStreaming() and also after a successful reconnect in reconnectStreaming();
ensure stopStreaming() and the disconnect callback still call
clipboardSyncService.stop() and set this.clipboardSyncService = null to avoid
leaks and duplicate observers.
In `@entry/src/main/ets/service/clipboard/ClipboardSyncService.ets`:
- Around line 99-106: The setupPasteboardListener currently always creates and
registers a new listener which can duplicate events if removePasteboardListener
failed; modify setupPasteboardListener to first check
this.pasteboardEventListener and the system pasteboard
(pasteboard.getSystemPasteboard()) and if a listener already exists either reuse
it (avoid creating a new closure) or explicitly call sysBoard.off('update',
this.pasteboardEventListener) before creating/registering a new one; ensure
removePasteboardListener also clears this.pasteboardEventListener only after
off() succeeds so the presence check works as intended and use the same
PasteboardUpdateListener reference for on/off calls.
- Around line 213-220: The code increments this.pendingSelfWrites before calling
pasteboard.getSystemPasteboard() and sysBoard.setDataSync(...) but does not roll
the counter back on failure; update the catch block in ClipboardSyncService
(symbols: this.pendingSelfWrites, pasteboard.getSystemPasteboard,
sysBoard.setDataSync, formatError, logError, this.observer?.onError) to
decrement/rollback this.pendingSelfWrites when an exception occurs (ensure it
won’t go negative), then proceed to log the error and notify observer as before
so failed writes don't leave a stale pendingSelfWrites count.
In `@entry/src/main/ets/service/SettingsService.ets`:
- Around line 63-66: The two clipboard setting constants
(ENABLE_CLIPBOARD_SYNC_TEXT and ENABLE_CLIPBOARD_SYNC_IMAGE) use camelCase
values and lack the file's settings_<snake_case> prefix; change their string
values to match the existing convention (e.g.
'settings_enable_clipboard_sync_text' and
'settings_enable_clipboard_sync_image') inside SettingsService.ets so they align
with other keys like 'settings_enable_vibration'; no other files should need
edits because callers use these constants, but perform the change before any
user data writes to avoid migration issues.
In `@entry/src/main/module.json5`:
- Around line 119-126: The code declares READ_PASTEBOARD but never requests it
at runtime; update ClipboardSyncService initialization to mirror the MICROPHONE
pattern (see StreamingSession.ets / MicrophoneManager.ets) by calling
checkAccessTokenSync for READ_PASTEBOARD and, when not granted, invoking
requestPermissionsFromUser to prompt the user and handle the async result before
any clipboard API usage; ensure callers in SettingsPageV2.ets and
CustomKeyOverlay.ets either await this initialization or check the granted state
so clipboard operations only run after permission is granted.
In `@nativelib/src/main/cpp/callbacks.cpp`:
- Around line 1005-1058: BridgeClClipboardData currently trusts remote-declared
payloadLength and allocates memcpy accordingly; add a protocol upper bound check
to avoid huge allocations. Before allocating or copying, compare
payloadLength/availablePayloadLength against a new or existing constant (e.g.
MAX_CLIPBOARD_PAYLOAD) and reject the frame if it exceeds the limit (log a
warning and free/delete cbData if allocated). Ensure you perform the check using
the parsed payloadLength and availablePayloadLength variables in
BridgeClClipboardData and skip malloc/memcpy (and avoid calling
napi_call_threadsafe_function) when over-limit, keeping all existing frees
(free(cbData->ptrParam) and delete cbData) on early exit.
In `@nativelib/src/main/cpp/moonlight_bridge.cpp`:
- Around line 171-190: GetByteArrayData currently treats the length returned by
napi_get_typedarray_info as bytes but that value is an element count and the
function accepts any TypedArray type; change the typed-array branch in
GetByteArrayData to call napi_get_typedarray_info (using the existing
napi_typedarray_type `type` and the returned element count), then compute the
actual byte length by multiplying element count by the element size for the
reported `type` (handle napi_uint8_array, napi_int8_array,
napi_uint8_clamped_array as 1 byte; handle int16/uint16 as 2 bytes;
int32/uint32/float32 as 4 bytes; float64 as 8 bytes; etc.), assign *length to
that byte length and set *data to the returned data pointer; optionally return
false (reject) for unsupported types instead of silently returning element
count. Ensure the arraybuffer branch still sets *length to the byte length
returned by napi_get_arraybuffer_info and leave the final fallback to set
*data=nullptr and *length=0.
---
Nitpick comments:
In `@docs/CLIPBOARD_SYNC_INTEGRATION_HARMONYOS.md`:
- Around line 154-165: Update the paragraph that begins "本地或远端一旦进入图片分支,只提示一次不支持"
to explicitly document the user-facing UX: include the exact downgrade message
text (provide Chinese and optional English), state the UI element used (e.g.,
system Toast/Snackbar vs modal) and its duration/priority, and define the
"只提示一次" scope (e.g., once per app session, once per device until user dismisses,
or persisted via a flag in local storage) plus how the flag is cleared; ensure
these additions appear immediately after the referenced sentence so readers can
find the concrete message, display mechanism, and the precise
persistence/clearing logic for the one-time notification.
🪄 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
Run ID: 6cc8f803-d417-4313-996f-77af9c75a5b6
📒 Files selected for processing (19)
docs/CLIPBOARD_SYNC_COMPARISON.mddocs/CLIPBOARD_SYNC_IMPLEMENTATION.mddocs/CLIPBOARD_SYNC_INTEGRATION_HARMONYOS.mdentry/src/main/ets/pages/SettingsPageV2.etsentry/src/main/ets/pages/StreamPage.etsentry/src/main/ets/service/SettingsService.etsentry/src/main/ets/service/clipboard/ClipboardSyncConfig.etsentry/src/main/ets/service/clipboard/ClipboardSyncService.etsentry/src/main/ets/service/jni/ClipboardBridge.etsentry/src/main/ets/service/streaming/MoonBridge.etsentry/src/main/ets/service/streaming/StreamingSession.etsentry/src/main/module.json5entry/src/main/resources/base/element/string.jsonnativelib/src/main/cpp/callbacks.cppnativelib/src/main/cpp/callbacks.hnativelib/src/main/cpp/moonlight-common-cnativelib/src/main/cpp/moonlight_bridge.cppnativelib/src/main/cpp/moonlight_bridge.hnativelib/src/main/cpp/napi_init.cpp
- restart clipboard sync after reconnect and gate startup on READ_PASTEBOARD - reuse pasteboard listener and roll back pendingSelfWrites on write failure - align clipboard settings keys with settings_<snake_case> naming - clamp inbound clipboard payload size and compute typed array byte lengths correctly - document one-time image downgrade toast UX
- verify StreamPage clipboard sync permission gating is already present - leave SettingsPageV2 unchanged because it only writes clipboard data - request READ_PASTEBOARD in CustomKeyOverlay before paste/import reads
- 修复客户端到主机文本剪贴板发送桥接 - 文本同步优先于图片,避免旧图片抢占 - 设置页禁用图片同步开关并清理旧配置 - 串流启动时强制关闭图片同步
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@entry/src/main/ets/pages/StreamPage.ets`:
- Around line 1458-1466: The code currently overwrites the user's persisted
image-sync preference by calling
PreferencesUtil.put(SettingsKeys.ENABLE_CLIPBOARD_SYNC_IMAGE, false) when
enableClipboardSyncImagePref is true; change this so we never persistently
modify the user's setting—remove the PreferencesUtil.put call and instead
enforce image sync being disabled only at runtime by using a local/runtime
variable (e.g., leave enableClipboardSyncImagePref as read-only and set
enableClipboardSyncImage = false for the current session) in the StreamPage.ets
flow so the user's SettingsKeys.ENABLE_CLIPBOARD_SYNC_IMAGE preference is not
overwritten.
🪄 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
Run ID: 112da417-9a11-4f8e-92e9-ca006b6ef31a
📒 Files selected for processing (9)
docs/CLIPBOARD_SYNC_INTEGRATION_HARMONYOS.mdentry/src/main/ets/components/CustomKeyOverlay.etsentry/src/main/ets/pages/SettingsPageV2.etsentry/src/main/ets/pages/StreamPage.etsentry/src/main/ets/service/SettingsService.etsentry/src/main/ets/service/clipboard/ClipboardSyncService.etsentry/src/main/ets/service/jni/ClipboardBridge.etsnativelib/src/main/cpp/callbacks.cppnativelib/src/main/cpp/moonlight_bridge.cpp
✅ Files skipped from review due to trivial changes (1)
- entry/src/main/ets/service/SettingsService.ets
🚧 Files skipped from review as they are similar to previous changes (3)
- nativelib/src/main/cpp/callbacks.cpp
- entry/src/main/ets/service/jni/ClipboardBridge.ets
- nativelib/src/main/cpp/moonlight_bridge.cpp
- NvHttp.uploadClipboardBlob/downloadClipboardBlob: 删除针对 Sunshine 不存在 的备选路径 (api/clipboard/blob, ?id= query 形式)。这些 fallback 每次都要 走完一轮 HTTPS 握手 + frp retry,污染日志且掩盖真实错误。Sunshine 是唯一 对接目标,路径固定 /api/v1/clipboard/blob。 - 收敛 upload response 解析为只读 id 字段,移除 ref/blobId/blob_id 等幻想 字段名兼容;非 JSON 响应直接抛错而非降级当作裸 id 使用。 - ClipboardBlobTransport.downloadBlob 接口由 (ref) 改为 (id),与 uploadBlob 返回字符串语义对称;transport 层无需 mime/size 元数据。 - ClipboardSyncService.onPasteboardChanged 删除末尾死代码。 - decodePngPayload 避免对已经是独占 buffer 的 Uint8Array 做多余拷贝。
- 反向依赖:NvHttp.uploadClipboardBlob 改为返回内部 ClipboardBlobUploadResult (id/mime/size 扁平结构),由 SunshineClipboardBlobTransport 在业务层包装为 ClipboardBlobRef。NvHttp 不再 import clipboard 模块类型,分层洁净。 - isTextMime 收紧:仅接受 text/* 且 charset 为 utf-8/utf8/us-ascii 或未指定, 避免 text/csv;charset=gbk 之类被按 UTF-8 解码造成乱码。 - decodePngPayload 始终拷贝 buffer,避免 ImageSource 异步持有 payload.buffer 在调用域之外的潜在 use-after-release 风险。 - 入站 4 处 (KIND_PNG/REF→PNG, KIND_TEXT/REF→TEXT) decode→pendingSelfWrites ++→write→catch--→onError 重复模式合并为 writeRemoteImagePayload / writeRemoteTextPayload 两个 helper,并修正自写计数泄漏 (claimedSelfWrite flag 确保仅在真正 ++ 后才 --)。
- ClipboardSyncConfig: 新增 ClipboardBlobRef / KIND_REF=3 / buildRefPayload / parseRefPayload,承载小 JSON 引用包 - ClipboardBridge: ClipboardKind enum 增 REF=3 - HttpClient: 新增 postBinary / postBinaryWithClientCert,支持 headers 自定义; TypedArray body 走 byteOffset/byteLength 切片,避免发送底层完整 buffer - SettingsPageV2: 重新放开图片剪贴板同步开关 - StreamPage: 仅在 image 同步开启时才构造 SunshineClipboardBlobTransport, 并在找不到 computer 时打 warn(不再静默 null 化)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
entry/src/main/ets/service/clipboard/ClipboardSyncService.ets (1)
112-115: ⚡ Quick win改进 pasteboard 事件监听的容错性。
当前 setupPasteboardListener() 方法的逻辑是:如果已注册过 listener,先调用 off() 清理再 on()。虽然 pasteboard 的 off() 方法对未注册的 listener 通常不会抛错(这是标准事件 API 的行为),但如果 off() 因其他系统错误而失败,整个 try-catch 会阻止 on() 执行,导致剪贴板监听被无声地禁用。
为增强容错性,建议将 off() 用 try-catch 单独包装,让它成为"尽力而为"的操作,确保 on() 始终能执行:
建议修改
} else { - sysBoard.off('update', this.pasteboardEventListener) + try { + sysBoard.off('update', this.pasteboardEventListener) + } catch (err) { + logWarn(`Failed to detach stale pasteboard listener: ${formatError(err as Object)}`) + } } sysBoard.on('update', this.pasteboardEventListener)这样即使旧监听清理失败,新监听仍会被注册,避免剪贴板同步功能完全失效。
🤖 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 `@entry/src/main/ets/service/clipboard/ClipboardSyncService.ets` around lines 112 - 115, The setupPasteboardListener() currently calls sysBoard.off('update', this.pasteboardEventListener) and then sysBoard.on('update', this.pasteboardEventListener) in the same try block; wrap the off call in its own try-catch so failures to remove an old listener do not prevent registering the new one—i.e., call sysBoard.off('update', this.pasteboardEventListener) inside a small try { ... } catch (err) { log or ignore } and ensure sysBoard.on('update', this.pasteboardEventListener) always runs afterward (outside that catch), keeping references to this.pasteboardEventListener unchanged.
🤖 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 `@entry/src/main/ets/service/clipboard/ClipboardSyncService.ets`:
- Around line 405-410: In ClipboardSyncService (the method that calls
blobTransport.downloadBlob(ref.id)) change the current soft size-mismatch log
into a hard failure: if ref.size is set and data.length !== ref.size,
reject/throw an error instead of returning data; additionally enforce an
independent maximum allowed blob size (e.g. a MAX_BLOB_BYTES constant) and
validate both the declared ref.size and the actual data.length against that cap
before returning. Update the logic around blobTransport.downloadBlob, ref.size,
and the code that returns data to perform these checks and to fail fast on
violations.
In `@entry/src/main/ets/utils/HttpClient.ets`:
- Around line 400-413: postBinaryWithClientCert is not forwarding
caller-provided headers (e.g., config?.headers) into the HttpClient.postBinary
call, causing headers like X-Clipboard-Mime to be dropped; fix by passing the
headers through in the options object (include headers: config?.headers) when
calling HttpClient.postBinary in postBinaryWithClientCert so the caller's
request headers are preserved alongside skipServerValidation, clientCertPath,
clientKeyPath, and timeout fields.
---
Nitpick comments:
In `@entry/src/main/ets/service/clipboard/ClipboardSyncService.ets`:
- Around line 112-115: The setupPasteboardListener() currently calls
sysBoard.off('update', this.pasteboardEventListener) and then
sysBoard.on('update', this.pasteboardEventListener) in the same try block; wrap
the off call in its own try-catch so failures to remove an old listener do not
prevent registering the new one—i.e., call sysBoard.off('update',
this.pasteboardEventListener) inside a small try { ... } catch (err) { log or
ignore } and ensure sysBoard.on('update', this.pasteboardEventListener) always
runs afterward (outside that catch), keeping references to
this.pasteboardEventListener unchanged.
🪄 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
Run ID: 01ee8fb4-6be5-40d2-9a2a-bf8ba2dadaea
📒 Files selected for processing (9)
entry/src/main/ets/pages/SettingsPageV2.etsentry/src/main/ets/pages/StreamPage.etsentry/src/main/ets/service/clipboard/ClipboardBlobTransport.etsentry/src/main/ets/service/clipboard/ClipboardSyncConfig.etsentry/src/main/ets/service/clipboard/ClipboardSyncService.etsentry/src/main/ets/service/clipboard/SunshineClipboardBlobTransport.etsentry/src/main/ets/service/jni/ClipboardBridge.etsentry/src/main/ets/service/streaming/NvHttp.etsentry/src/main/ets/utils/HttpClient.ets
BridgeArInit 分配 g_decodedAudioBuffer 前先 free 旧 buffer: - 避免上轮 BridgeArCleanup 未完整走完时的内存泄漏 - 防止声道数变化(5.1 ↔ 2.0)时 size 不一致越界
BridgeArDecodeAndPlaySample 增加诊断采样: - 每 ~1s(200 帧)打一行 [AUDIO_DIAG] 窗口统计: frames / samples / maxAbs / meanAbs / 饱和率 / bass 触发数 / intensity 峰值 / decode 失败累计 - 单帧饱和率 > 50% 立即 WARN - decode 返回 <= 0 时打 WARN(前 3 次每次 + 之后每 50 次抽样) - 与 BridgeArInit / Cleanup / Start / Stop 已有 INFO 一起,可按时间轴对位 - 用户复现海生哥反馈的'断连重连卡爆音 + 手柄乱震'后,hilog grep [AUDIO_DIAG] 即可定位是否爆音、爆音特征(饱和/解码失败/intensity 突高)
Two upstream NULL deref crashes observed in AGC reports (100+ records): - RtpvAddPacket+0xC00: pendingFecBlockList.head NULL in reconstructFrame cleanup_packets path -> SIGSEGV @ 0x10 on VideoRecv thread. - RtpaGetQueuedPacket+0x138: dataPackets[i] NULL -> memcpy SIGSEGV @ 0 on AudioRecv thread. Submodule pointer: 3dde0f1 -> 2bceac6 (mic branch).
CodeRabbit review 反馈: - ClipboardSyncService.downloadClipboardBlob 仅在 size 不匹配时记日志便继续解码, 恶意/异常服务端可借此触发客户端 OOM。改为在下载前/后做硬性 64 MiB 上限校验, 并把 size mismatch 升级为 hard fail。 - HttpClient.postWithClientCert / postBinaryWithClientCert 构造 inline config 时 漏传 config?.headers,会导致上层(如 X-Clipboard-Mime)传入的请求头被静默 丢弃。
Summary
Validation
Notes
Summary by CodeRabbit
新功能
文档