Repository navigation
fix: resolve 48 findings from the cross-validated code review (#4) - #5
Conversation
…ktop#4) Batch fix for the review checklist in UniDesktop#4. The two items already fixed in v0.2.1 (P0-2 dbusmenu wire type, P1-23 SNI Menu object path) are untouched; everything else lands here, including the production-side completion of P2-50 (Partial(reason)). Highlights by area: Engineering gate (P0-1) - uda-ffi compiles on non-Linux/Windows targets again: cfg-fallback appearance()/wallpaper() backends, gated tokio use in notify, missing UdaError import, unused imports. Windows tray (P0-4/5/6, P1-24~29, P2-41~44, P2-33, P1-10) - hide/show round-trip (dwStateMask=NIS_HIDDEN), every GDI bitmap released on all paths, two-phase show_menu removes the &mut aliasing UB across the modal pump, per-worker reentrancy guard, TaskbarCreated re-registration on a hidden top-level window, NIF_SHOWTIP, NIN_SELECT/NIN_KEYSELECT, NIM_MODIFY failure no longer leaks the new HICON, caches advance only on success, AppendMenuW results checked, AUMID early-return no longer leaks CoTaskMem, empty wallpaper reads as NULL, refused media commands report NotSupported. Linux platform (P0-3, P1-15/19/21, P2-39/40, P1-10) - hyprpaper driven via hyprctl, swww daemon probe/spawn with timeouts, no silent Hyprland fallback; nested-block_on panic fixed via a try_current bridge; per-DE logout signatures and the correct XFCE names; GNOME 47 accent palette mapping with capability probing; timeouts on all D-Bus round trips; get_wallpaper implemented for GNOME with the capability bit aligned. Linux tray (P1-16~22, P2-36~38, P2-50) - readiness handshake fails create() instead of advertising a dead icon; menu fingerprinting with real LayoutUpdated emission; exact parent_id/recursionDepth semantics; stable dbusmenu id map; root node standard properties; panic-safe handler restore; second click falls back to on_click; icon transcode cache; honest Partial(reason) support levels; NewToolTip signal. FFI contract (P0-7 FFI side, P1-8~14/32, P2-33/34/35/48) - destroy_menu detaches the menu from every live icon before the registry record goes; last-error message served from a thread-local borrow (no per-failure leak, docs aligned across lib/uda.h/bindings); set_action failures reported; Weak trampoline capture; push ids used directly; single-lock type checks; InvalidArgument variant mapped to UDA_ERR_INVALID_ARGUMENT; empty strings export as NULL; lazy wakelock reaping; GNOME-only Tier-3 wallpaper probe; new uda_tray_capabilities export. Bindings & CLI (P0-7 binding side, P1-30/31, P2-45~47/49) - Node destroy() unregisters trampolines only after the FFI destroy; single-call wallpaper read; explicit library path honoured; failed row adds release their koffi slots; Python passes the real status code and releases empty strings; uda-cli loses its unwraps, hardcoded path, and side effects (--dry-run, argument parsing, cfg gating). Verification: cargo check --workspace --all-targets clean on macOS, x86_64-pc-windows-msvc and x86_64-unknown-linux-gnu; 58 + 110 + 116 tests pass; header/export symbol check 33/33.
The transport test swapped the menu and immediately clicked dbusmenu id 1. Under the stable id map that id belongs to the previous layout and stays retired, so the click landed on no row and the counter stayed at zero - the CI run with a real session bus exposed it. The test now pins both halves of the contract: a click on the stale id is dropped, and a click on the id handed out by a fresh GetLayout read fires. Also applies the rustfmt cleanups the lint job flagged.
There was a problem hiding this comment.
审查完毕，存在问题需要修复
审查过程包含离线测试。由于出现问题，本次审查未进行实机测试。
本次审查对象为来自 @umbrella22 的 2 commits based on tag v0.2.1（包含 47 文件 和 +6200 / −1664 变更）
摘要
本 PR 在工程实现层面质量显著高于社区平均水准，且全量编译 / 测试 / 交叉编译 / 符号一致性检查全部真实通过。但 3 项阻塞性问题破坏性契约变更，按生产环境零事故标准不予放行。修复阻塞性问题并补齐后，本 PR 具备合并条件。
阻塞性问题（High or Higer，影响 merge）
1. uda_last_error_message() 所有权语义翻转（ABI 运行期断裂）
位于
crates/uda-ffi/src/util.rs:60-91、include/uda.h:504-517
uda_last_error_message() 的所有权语义被翻转：由"新分配、调用方必须 uda_free_string()"改为"借用库所有的线程局部缓冲区、禁止 free"。这是运行期破坏性变更——旧宿主继续 free() 将触发堆损坏；且缓冲区在下一次同一线程的调用即被覆写，旧宿主若延迟读取会拿到错误文本
2. Theme / SupportLevel 破坏性变体变更，但无 #[non_exhaustive] （存疑）
位于
crates/uda-core/src/capability.rs:86-101、crates/uda-core/src/capability.rs:103-Theme与SupportLevel两个 pub
enum 做了破坏性签名变更：SupportLevel::Partial 由 unit variant 变为 Partial(String)（同时移除 Copy）；Theme 新增 Unknown 变体。这是正确的，但问题在于未标 #[non_exhaustive]。任何下游 crate 对这两个枚举做穷尽 match 或依赖 Partial 的 Copy 语义，升级后必然编译失败。
3. Windows 工作窗口缺 WS_EX_TOOLWINDOW，产生幽灵窗口
位于
crates/uda-platform-windows/src/tray.rs:95、crates/uda-platform-windows/src/tray.rs:99、crates/uda-platform-windows/src/tray.rs:1092-1114
Windows 隐臧工作窗口缺少 WS_EX_TOOLWINDOW（WINDOW_EX_STYLE_BITS = WINDOW_EX_STYLE(0)）却使用 WS_OVERLAPPEDWINDOW 样式。该窗口虽为 0×0 且无 WS_VISIBLE，但仍会在 Alt+Tab 切换列表、任务管理器"进程/窗口"视图、EnumWindows 枚举中作为可枚举顶层窗口出现，形成"幽灵窗口"
二级问题（Medium，强烈建议修复）
1. CHANGELOG.md 缺失本 PR 条目
位于
CHANGELOG.md:1-58
本 PR 是"一次性修复 48 项缺陷"的大改动，但 CHANGELOG 的整个 diff 只是在 main 基础上补写 v0.2.1 已发布节点（CHANGELOG.md:12-49）与修复链接定义（CHANGELOG.md:113-115）。[Unreleased] 链接引用仍在（CHANGELOG.md:113）但没有任何 [Unreleased] 内容节，48 项修复全部无记录。更严重的是 CHANGELOG.md:48-49 写明"示例代码无需调整：本次为修复性补丁，未新增或改动任何 API"——这句断言对本 PR 是虚假的（本 PR 改动了 SupportLevel、Theme、UdaError 变体、FFI 签名）。
2. 17 个 FFI 导出由 extern "C" 改为 unsafe extern "C" 未做兼容性说明
位于
crates/uda-ffi/src/lib.rs（24 → 25 处pub unsafe extern "C" fn，整体导出 33 个中 25 个为 unsafe）
main 基线为 8 safe + 24 unsafe；本 PR 后为 8 safe + 25 unsafe，即约 17 个原本 safe 的导出被改为 unsafe。对 C 调用方无影响（unsafe 不影响 C ABI 与链接），但对 Rust 侧直接 extern "C" 声明调用的宿主是编译期破坏。
3. 托盘窗口过程缺 WM_NCDESTROY 的 GWLP_USERDATA 清理
位于
crates/uda-platform-windows/src/tray.rs:1824-1859（窗口过程全文无WM_DESTROY/WM_NCDESTROY/PostQuitMessage分支）
当前设计靠 teardown() 主动 DestroyWindow（:1677）后由 run() 退出循环（:1392-1394），自洽。但窗口过程完全没有 WM_NCDESTROY 分支清空 GWLP_USERDATA（:1129 写入的 *mut Worker）。若因任何外部原因（宿主进程被 EndTask、调试器、或未来新增的提前 DestroyWindow 路径）窗口先于 worker 销毁，窗口过程将继续解引用已悬空的 Worker 指针。同理缺 WM_DESTROY → PostQuitMessage 使消息循环生命周期只依赖 on_tick 返回值，闭环路径单一。
4. cargo doc 新增 8 条 rustdoc 警告
位于
crates/uda-platform-windows/src/lib.rs:3（cfg(windows)）、crates/uda-platform-windows/src/lib.rs:11-16（appearance/wallpaper/wakelock/detection/notification/tray）、crates/uda-core/src/notification.rs:119（public doc 链接 privateis_absolute_path）
在 #[cfg(windows)] 门控下编译时模块不可见，导致模块级清单的 intra-doc 链接无法解析；uda-core/src/notification.rs:119 的公开文档链到了私有函数。
三级问题（Low，建议修改）
1. WINDOW_STYLE_BITS 过宽
位于
crates/uda-platform-windows/src/tray.rs:95
即便加上 WS_EX_TOOLWINDOW，WS_OVERLAPPEDWINDOW 仍带 WS_CAPTION | WS_SYSMENU | WS_THICKFRAME | WS_MINIMIZEBOX | WS_MAXIMIZEBOX，对"永不可见"的窗口是多余 surface。WS_POPUP 已足够接收广播。
2. sync.rs 无条件预构建 runtime
位于
crates/uda-platform-linux/src/sync.rs:43-52
tokio::runtime::Builder...build() 在 Handle::try_current() 判定之前执行，导致 ambient-runtime 分支下构建出的 runtime 还要移动到工作线程。功能正确（与 FFI 侧 run_sync 行为等价），但顺序不如 crates/uda-ffi/src/notify.rs:110-113（先判定后构建）。
本 Comment 由人工编写，审查建议中包含 AI 给予的建议。
…ndow Review round on PR UniDesktop#5 (CHANGES_REQUESTED). Blocking items: - uda_last_error_message returns a caller-owned allocation again: the borrow-the-thread-local-buffer contract this branch introduced was a runtime ABI break for hosts that (per the previous header) free the string. Ownership semantics are now byte-for-byte what v0.2.1 shipped; only the previously contradictory docs are aligned, and both bindings release the string through uda_free_string after a single in-place decode. - Theme, SupportLevel and UdaError are #[non_exhaustive]: these enums gained variants across patch releases, so downstream exhaustive matches stop being compile-time breaks. Cross-crate matches in the workspace carry sensible wildcard arms. - The Windows worker window gets WS_EX_TOOLWINDOW | WS_EX_NOACTIVATE so it stops appearing in Alt+Tab, Task Manager and EnumWindows. Medium items: CHANGELOG gains an [Unreleased] section covering all 48 fixes plus an unsafe-extern compatibility note (the extern flips shipped in v0.2.1, not here); WM_NCDESTROY now clears GWLP_USERDATA and posts a quit so an externally destroyed window cannot leave a dangling worker pointer; the remaining rustdoc warnings (private-item links, cfg-gated module lists) are gone - cargo doc --no-deps is warning-free. Low items: the worker window drops to WS_POPUP (the caption/frame bits of WS_OVERLAPPEDWINDOW were dead surface), and run_async branches on the ambient runtime before building one, matching the FFI-side run_sync.
…uracy Cross-validation findings on the maintainer-review fixes: - Node: decoding the error-message pointer with koffi 'char *' read a pointer AT the address and crashed the process (SIGSEGV) on the first failure; decode in place via koffi's NUL-terminated char read instead. readSlot grows an optional length argument, byte-identical for every existing call site. - Python: c_void_p addresses cannot pass through the c_char_p argtype of uda_free_string (ArgumentError masked the real error); the argtype is now c_void_p, which accepts both raw addresses and the slot objects the wallpaper and media paths pass. - sync.rs: the worker-branch runtime-build failure propagated as UdaError::Io instead of the documented Internal, and skipped the worker join; the worker now maps the failure to Internal with the inline branch's wording and the caller always reaps. - Comment and changelog accuracy: WS_EX_TOOLWINDOW excludes Alt+Tab and the taskbar but not EnumWindows (that needs its own filter); the FFI exports were never flipped to unsafe extern - pointer-taking exports were introduced that way, so the compatibility note now states the actual history. - Last-error tests strengthened: pointers must differ across renders, the first allocation must survive the second render, and the public uda_free_string export is exercised end to end.
…ase symmetry Final validator findings on the review round: - CHANGELOG: the four findings that had no entry of their own (the cross-compile build gate, the last-error ownership contract itself, the media refusal status code, and the registry double-lock window) are now recorded, and the unsafe-extern note no longer implies that absence of pointer parameters is what makes an export plain. - crates/uda-ffi/src/error.rs: the module doc still described the previous borrow-until-next-call contract; it now matches the caller-owned one used everywhere else. - Node: the error-message pointer is released in a finally block, so a decode failure cannot skip the free; the koffi version comment now names the version the lockfile actually pins (2.x) instead of 3.x. - Python: a contrast note at the uda_status_message declaration records why that one keeps the c_char_p restype (a static string, never freed).
关于审查建议的修正关于
|
|
你可以在彻底完成一次修复后通过在 Comment 中 Mention 我，以通知我的审查。 |
…ntract Follow-up to the maintainer review's unsafe-extern finding. The finding rested on a comparison against a pre-v0.2.0 baseline: no export was ever flipped - pointer-taking exports were unsafe from introduction (v0.2.0) and the eight value-only exports have always been plain. The unsafe on pointer-taking exports is sound: a Rust caller passing a dangling non-null pointer is UB, which a safe function cannot accept, so downgrading them would trade correctness for appearances. What this adds instead is the guarantee the review actually wanted - no surprise reclassification: - export_safety_classification_is_pinned scans the crate source and asserts the exact value-only set (8 exports) against every other export being unsafe, and that the surface still totals 33. Moving an export between extern "C" and unsafe extern "C" now fails CI unless the contract list and the changelog are updated deliberately. - The changelog compatibility note now states the caller impact precisely: extern-block Rust hosts are unaffected on every edition (a definition-side unsafe keyword cannot break a block declaration); only rlib consumers see the unsafe requirement, unchanged since v0.2.0.
Cross-validation on the classification pin: the constant's doc claimed every export outside the list dereferences caller memory, but seven exports (the session controls and the media command) take no pointer and are still unsafe by policy. The list is now PLAIN_EXTERN_C_EXPORTS and its doc states it pins the classification rather than deriving it; the scanner drops an unreachable default branch so a malformed signature would fail loudly instead of being skipped.
|
感谢细致的审查,9 项意见已全部处理完毕,5 个新提交( 阻塞性问题1. 2. 3. Windows 工作窗口幽灵化 —— 已加 二级问题
三级问题
验证除 CI 四项(Lint / Linux / Windows / Cross-Compile)全绿外,本机还在 |
There was a problem hiding this comment.
审查完毕，达成合并条件
审查过程包含离线测试 + Windows Subsystem for Linux 2 (Ubuntu) 中无头环境的实机测试
本次审查对象为来自 @umbrella22 的 7 commits based on tag v0.2.1
摘要
本 PR 满足 v0.2.2 bugfix 的合并门槛，上一轮提出的 3 项 P0（uda_last_error_message() 所有权语义翻转、Theme/SupportLevel 缺 #[non_exhaustive]、Windows 幽灵窗口）与全部 P1/P2 均已修复并复测通过，未引入任何 semver-breaking 公共 API 变更。
但本次跟进审查（WSL 无头环境全示例实跑）暴露了 5 项非阻塞性技术债务与事实偏差。以下逐项列明证据跟进与修复的必要性。
存在问题（Medium or higher，建议修复）
1. Theme 检测在无头环境返回错误的 light
位于
crates/uda-platform-linux/src/appearance.rs
parse_theme_from_gtk_theme 没有 None 出口——任何非 -dark 后缀一律判 Light。WSL 中该 schema 由 GTK 包装带入，gtk-theme 取的是从未被设置过的 schema 默认值 'Adwaita'（本机无 dconf 后端）。函数把一个无人配置过的默认值当成了确定答案。这违反了 appearance.rs:27-30 的设计意图。
2. wakelock 的 Tier-3 兜底可能完全无效，宿主无从知晓
位于
crates/uda-ffi/src/wakelocks.rs
机制层面 wakelocks.rs:167-192 的兜底、3600s 上限、到期回收与 kill+reap 都真实闭环。但 WSL 无 systemd-logind，没有任何组件受理该 inhibit 锁——屏幕该休眠照样休眠，而宿主收到的是 UDA_OK。
3. tray 降级状态对宿主不可见
位于
crates/uda-platform-linux/src/tray.rs
降级原因仅进 log::warn，而 log crate 无默认实现，宿主永远看不到（实测 RUST_LOG=debug 输出为空正是此因）。这是符合 tray_specs.md §1.6 的有意设计（watcher 可能后启动，item 先导出），取舍正确。然而 WSL 下托盘图标不可能显示，宿主却收到成功。SupportLevel::Partial(reason) 机制本 PR 已经就绪，只差把它暴露给宿主。
4. 07_session 能力矩阵无任何探测即宣称全支持
位于
examples/python/07_session.py和examples/nodejs/07_session.js
WSL（无 org.freedesktop.login1、无 ScreenSaver、无 session manager）实跑 07_session，六项动作全部列出"支持"，未做任何一次 D-Bus 可达性探测，直接打印硬编码说明文档。对比 02_wallpaper/03_notification 会真实调用并报错、06_media 明确标注"这不算错误"，07_session 是全套示例中唯一无条件宣称成功的。
5. Python 与 Node.js 错误语义分叉
位于
crates/uda-ffi
这是本 PR 波及的绑定层行为差异，且 PR 的 48 项清单虽包含 Node 侧修复（new Uda(path) 参数丢弃、_readWallpaper 双调用、koffi 槽位泄漏），却未统一错误退出语义。宿主集成预期会因此不一致。
本 Comment 由人工编写，审查建议中包含 AI 给予的建议。
The maintainer's WSL2 follow-up review confirmed all blocking and P1/P2 findings fixed, and raised five non-blocking honesty gaps: - Theme detection no longer treats a never-configured gtk-theme schema default as a light answer: a -dark suffix answers Dark, a Light answer requires dconf read to prove the key was explicitly set, and everything else falls to Theme::Unknown. - The wakelock CLI fallback verifies org.freedesktop.login1 on the system bus before spawning systemd-inhibit; an unlockable lock now fails with NotSupported instead of reporting success. Wake-lock capabilities and support_level are driven by real reachability, reporting Partial(reason) when only a degraded tier is reachable. - Tray degradation becomes host-visible: the degraded reason recorded at worker readiness (a missing watcher) now flows into the icon state, LinuxTrayManager::support_level reports Partial(reason), and two new exports (uda_tray_support_level / uda_tray_support_reason with the UDA_TRAY_SUPPORT_LEVEL_* constants) let hosts query level and reason per handle. Windows reports FULL / NULL. - 07_session (Python and Node) probes uda_session_capabilities first and renders the support matrix from the probe, instead of printing a hardcoded all-supported table. - The Node binding gains a UdaError class carrying the numeric status plus exported status-code constants, aligning error semantics with the Python binding. Verified across macOS / windows-msvc / linux-gnu targets (0/0), the full suite under a real D-Bus session (304 tests, including the new watcherless-degradation transport test), cargo doc and clippy clean.
Cross-validation follow-ups on the headless-honesty round: - The KDE and XFCE theme fallbacks had the same guess-Light leak the GNOME path just lost: any non-dark scheme name answered Light, and tooling-written defaults (Breeze, Greybird) would masquerade as a configured light theme. Both now share one dark-name-or-unknown rule, and XFCE reads the actual theme key (xsettings /Net/ThemeName) rather than the desktop backdrop mode it used to query. - TrayIcon::set_degraded normalises empty reasons to None: uda.h promises PARTIAL always carries a non-empty explanation, and an empty string would have surfaced as PARTIAL with a NULL reason. - The Node binding's macOS library candidates include libuda_ffi.dylib, so the documented 'cargo build && node examples/nodejs/07_session.js' flow works without setting UDA_LIBRARY. - Python __all__ exports the status-code constants, matching the Node module.exports surface. AGENTS.md was checked for the gen-header drift the validators flagged: the on-disk file makes no such reference, so no change was needed.
|
@SRInternet 第二轮复审的 5 项意见已全部修复完毕，新提交 1. Theme 检测在无头环境返回错误的 Light —— 2. wakelock 的 Tier-3 兜底可能完全无效 —— 3. tray 降级状态对宿主不可见 —— 按你的建议直接用了已就绪的 4. 07_session 无探测即宣称全支持 —— Python 与 Node 两个示例都改为启动即查询 5. Python 与 Node.js 错误语义分叉 —— Node 新增 交叉验证额外收口（验证代理发现，一并修了）：KDE / XFCE 的 theme 回退存在与 GNOME 同类的"猜 Light"旁路（工具写入的 Breeze/Greybird 默认值会被当答案），已统一为"深色名才答 Dark、否则 Unknown"；XFCE 原来查询的 验证：macOS / windows-msvc / linux-gnu 三目标 |
Final polish from the cross-validation pass on the headless-honesty
round (comment/doc/test only, no behaviour change):
- The wake-lock support_level doc states precisely which layer the
Partial tier describes: this manager's own acquire speaks ScreenSaver
only; the systemd-inhibit CLI tier the reason points at lives in
uda-ffi's registry.
- The appearance module's fallback-chain list now names the XFCE key
the code actually reads (xsettings Net/ThemeName).
- tray_specs.md 1.7 marks Show icon Partial when the watcher is absent,
matching the new degradation reporting.
- The Node binding picks its library filename from a platform map
(mirroring the Python binding) instead of a nested ternary, and the
British spelling and blank-reason wording are evened out.
- A regression test pins TrayIcon::set_degraded's blank-reason
normalisation (Some("") and whitespace land as None / Full), and a
redundant hand-paired assertion is gone.
There was a problem hiding this comment.
审查完毕，达成合并条件
审查过程包含离线测试 + Windows Subsystem for Linux 2 (Ubuntu) 中无头环境的实机测试
本次审查对象为来自 @umbrella22 的 10 commits based on tag v0.2.1
摘要
本 PR 满足 v0.2.2 bugfix 的合并门槛，本轮新增两 commit 精准响应了此前提出的 5 个二级问题，且其中两项已 WSL 实机证实确已修复。未发现新的阻塞性缺陷。
但本次跟进审查（WSL 无头环境全示例实跑）依旧暴露了 3 项非阻塞性技术债务与事实偏差。以下逐项列明证据跟进与修复的必要性。
存在问题（Medium or higher，建议修复）
1. Python/Node 示例错误退出语义仍未统一（exit 0 vs 1）
位于
examples/{python,nodejs}/*
进程退出语义仍未统一——Python 示例捕获后 exit=0，Node 示例仍为未捕获异常 exit=1。这是示例层选择，非库契约问题。
2. 不得以"进程/服务存在"替代"该功能在本环境真正可用"作为判断依据
位于
crates/uda-platform-linux/src/session.rs:477-490
零运行时探测，硬编码返回全部 7 个能力位。 这是 79b672f 宣称"07_session 改为实时探测"后唯一未被改到的模块——该 commit 只改了示例文件（examples/{python,nodejs}/07_session.*）去读取 uda_session_capabilities，却没有改后端 capabilities() 本身去真实探测。
3. 错误码与 uda.h 契约不符
位于
include/uda.h:421-425
include/uda.h:421-425 明确承诺：
A set bit means "the code path exists", not "the account is allowed": a machine with hibernation switched off still reports UDA_SESSION_CAP_HIBERNATE, and the attempt then fails with UDA_ERR_NOT_SUPPORTED.
实测却返回 -5 UDA_ERR_INTERNAL（Command failed: loginctl lock-session exited with 1），来自 session.rs:226-228 的 UdaError::CommandFailed。
uda.h 承诺 -2，实际给 -5。宿主按契约写 if (status == UDA_ERR_NOT_SUPPORTED) 降级，会漏判这种情况。这是比能力位误报更实的契约违反。
本 Comment 由人工编写，审查建议中包含 AI 给予的建议。
|
请先同步分支 develop 最新内容后继续变更，并对分支 develop 发起 PR 而不是分支 main。 |
Third-review round on PR UniDesktop#5: - LinuxSessionManager::capabilities no longer hardcodes the seven session bits. The matrix is rendered from runtime reachability: org.freedesktop.login1 on the system bus answers the full set, org.freedesktop.ScreenSaver alone answers LOCK only, and neither (a WSL headless session) answers honestly with an empty set. The probe is cached per process; failures fold to absent, the same conservative direction as the wake-lock and accent-colour probes. The capability-bit paragraph in include/uda.h now describes the probed semantics. - Session attempt failures report UDA_ERR_NOT_SUPPORTED (-2) as the header promises, not UDA_ERR_INTERNAL (-5): loginctl non-zero exits keep their diagnostics ('loginctl lock-session exited with 1'), polkit refusals, missing ScreenSaver services and missing CLIs all land on the single not-delivered constructor. A contract test enumerates every mapped path. - The Node examples render failures exactly like the Python _bootstrap: one 'UDA 调用失败: …' line and exit 1 when the environment cannot answer (previously an uncaught stack trace), while 07_session keeps 'probe throws -> 1, everything unsupported -> 0' on both sides.
Follows the round-3 validation pass on 2e9ebf9: - session.rs reuses wakelock::screensaver_present instead of carrying a verbatim copy, which also puts bus_has_service back to private; the review-round narration comments are trimmed to the constraints that outlive the review. - The probed capability semantics 2e9ebf9 put into include/uda.h are now stated consistently by every source: the Rust doc comments (uda-ffi lib.rs, uda-core session.rs), session_specs.md section 5 and the capability table, the 07_session examples' copy, and the session.capabilities docstrings in the python/nodejs bindings. The 'static query' phrasing becomes 'probed once, cached', because the Linux backend answers from a cached ListNames probe. - The module doc's dash list tripped clippy's doc_lazy_continuation under -D warnings (the Lint & Format job); it is a plain parenthetical now.
|
@SRInternet 第三轮的三项问题已全部修复，并完成了交叉验证与文档一致性收尾。 1. 能力位真实探测（
探测按进程缓存一次，失败一律按"不可达"收敛（与 wake-lock / accent 探测同一保守方向）。 2. 错误码收敛到 -2（2e9ebf9） — 所有"动作未送达"路径（logind 拒绝/超时、polkit 拒绝、 3. Node 示例退出语义（2e9ebf9） — 01/02/03/06 统一为顶层 try/catch → 一行 验证收尾（f9e5d84）：
本地验证： |
Docker-based headless-Linux verification (session bus present, no ScreenSaver, no logind - the shape dbus-run-session reproduces and a WSL host approximates) exposed two places still assuming the hardcoded-capabilities world: - the_capability_query_neither_errors_nor_lies asserted that Linux always reports SESSION_MANAGEMENT. Under the probed matrix a headless host honestly answers empty and a screen-saver-only host answers LOCK without the management bit, so the old assertion contradicted the very semantics this PR ships. It now pins the three legal probe states (full set / LOCK alone / empty) instead. - verify_someone_receives_the_lock propagated a failed logind probe as UDA_ERR_INTERNAL (-5), while its own contract says a failed probe refuses like an absent receiver. The refusal now carries UDA_ERR_NOT_SUPPORTED (-2) with the probe's diagnostics kept in the message, mirroring the session attempts' not-delivered mapping. Verified in a rust:1 container: uda-ffi 119 passed (was 117 passed + 2 environment failures) and platform-linux 135 passed under dbus-run-session; clippy --workspace --all-targets -D warnings clean.
|
@SRInternet 补充：推送 f9e5d84 后，我们在 Docker（rust:1 容器）里做了一轮无头 Linux 实测 — 容器内有会话总线、无 ScreenSaver、无 logind，正是
容器实测（修复后）： |
SRInternet
left a comment
SRInternet
left a comment
There was a problem hiding this comment.
审查完毕，达成合并条件
本次审查过程为离线测试和 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试 + 本地 Linux/Windows 双目标交叉编译与 dbus 沙箱自动化测试
审查对象包括来自 @umbrella22 的 13 commits target v0.2.2
摘要
本 PR 在"无头环境的诚实回答"这一主题上一定程度的完成了，其中 锁屏失败: UDA error -2 一行证明 include/uda.h 承诺的"按 -2 降级"契约已成立。剩余三项均为非阻塞的探测粒度与文档规范问题，不影响 v0.2.2 合入。
存在问题 (Medium or higher, 建议修复)
- session 能力探测粒度停在 service 可达层，未下沉到 session 对象可用层 (契约不符/能力位过度承诺)
位于
crates/uda-platform-linux/src/wakelock.rs:85-99crates/uda-platform-linux/src/wakelock.rs:110-136crates/uda-platform-linux/src/session.rs:516-526crates/uda-platform-linux/src/session.rs:555-569crates/uda-platform-linux/src/session.rs:606-612
logind_present() 以 DBusProxy::list_names() 判定 org.freedesktop.login1 是否在系统总线上被 own，session_capability_set() 据此把 logind=true 映射为全部 7 个能力位。但"服务名被 own"与"该服务在此 seat 上具备可用的 session 对象"是两个不同层级的命题：logind 是常驻 daemon，只要 systemd 起进程就会 own 该名字；而 loginctl lock-session 实际调用的是 /org/freedesktop/login1/session/auto，该对象只在真实 seat 会话建立时创建。WSL2 恰好落在这一夹缝——logind 进程在跑、bus 名被 own（审阅者以 busctl --system list 实测确认 org.freedesktop.login1 184 systemd-logind root :1.1 systemd-logind.service），却不存在任何 seat/session 对象。
logind_present()文档注释对 WSL 的事实陈述与实测相反 (文档误导/事实基准错误)
位于
crates/uda-platform-linux/src/wakelock.rs:78-84
注释原文写 "hand the host a lock that is pure fiction (observed on WSL, which exports no logind)"。审阅者在本轮审查中以 busctl --system list 实测确认 WSL2 确实导出 org.freedesktop.login1（由 systemd-logind.service 提供），缺失的是 session 对象而非 logind 本身。该断言把 WSL 归类为"无 logind"环境，与 session_capability_set() 注释中"With neither reachable - the WSL case"的表述（crates/uda-platform-linux/src/session.rs:553-554）自相矛盾：若 WSL 真的两个 receiver 都无，则能力位应为空集，而实机输出是全集。
三级问题 (Low, 建议规范)
cargo doc --document-private-items产生 12 条 intra-doc 链接告警 (文档规范)
位于
crates/uda-ffi/src/media.rs:12-7crates/uda-ffi/src/media.rs:21-36crates/uda-ffi/src/session.rs:15-9crates/uda-ffi/src/session.rs:20-12crates/uda-ffi/src/session.rs:26-7crates/uda-ffi/src/session.rs:52-34crates/uda-ffi/src/lib.rs:1008-54crates/uda-ffi/src/lib.rs:1025-54crates/uda-ffi/src/util.rs:141-23crates/uda-platform-linux/src/tray.rs:667-7
模块级文档以 intra-doc 链接语法 [uda_media_get_status]、[session_capabilities]、[UDA_SESSION_CAP_*]、[FillMode]、[WakeLockType]、[TrayIconInner] 引用 C-ABI 导出名或跨 crate 私有条目，rustdoc 无法在 uda-ffi 与 uda-platform-linux 的命名空间中解析它们；util.rs:141 另有一条 redundant explicit link target（[UDA_ERR_PANIC] 的目标与文本重复）。
本 Comment 由人工编写，审查建议中包含 AI 给予的建议。
Fourth review round on PR UniDesktop#5. Owning org.freedesktop.login1 on the system bus only proves the daemon runs: systemd starts it on every booted host, including a systemd-enabled WSL2 install where no seat session is ever established - the reviewer's busctl probe found the name owned there, so the matrix claimed all seven bits on a host where loginctl lock-session has no session object to speak to. The session matrix now asks the manager itself: a new logind_session_present() probe calls ListSessions() and insists on at least one live seat session before advertising the session-management bits. A logind without sessions (the WSL2 shape) honestly answers with the manager-level power actions alone; the wake-lock CLI-tier guard keeps using the manager-level check on purpose, since Inhibit needs the daemon, not a seat session. The WSL mentions that claimed the opposite of the reviewer's busctl finding are corrected: WSL with systemd disabled has no logind, while systemd-enabled WSL2 has the daemon but no seat session; the neither-reached sandboxes are containers and dbus-run-session, not WSL. Also fixes the rustdoc warnings that cargo doc --document-private-items reports in uda-ffi and the Linux tray (broken intra-doc links to C-ABI export names and cross-crate items, one redundant explicit target), so the doc build is warning-free. Verified in a rust:1 container against a mock logind on a real system bus: the matrix answers power-actions-only with zero sessions, the full set after AddSession, power-only again after RemoveSession, LOCK with a ScreenSaver alone, and empty in a bare dbus-run-session; plus the full battery (mock script, uda-ffi 119, clippy -D warnings, fmt, 0 rustdoc warnings).
|
@SRInternet 第四轮的三项已全部处理，提交为 498e1c0： 1. 探测下沉到会话对象层 认同"名字被 own ≠ seat 会话可用"。新增
wakelock 的 CLI 层守卫有意保留 manager 级探针： Docker 实测（rust:1 容器，自制 logind mock 挂在真实 system bus 上，可动态 Add/RemoveSession）：0 会话 → 仅电源位 ✓；AddSession → 全集 ✓；RemoveSession → 回落仅电源位 ✓；仅 ScreenSaver → LOCK ✓；裸 2. WSL 事实陈述修正 按您的 busctl 实测改写了全部相关注释：启用 systemd 的 WSL2 确实运行 logind，"WSL 无 logind"的旧说法只适用于 systemd 被禁用的 WSL 配置； 3. rustdoc 告警清零
验证：容器内 mock 脚本全绿、 |
SRInternet
left a comment
SRInternet
left a comment
There was a problem hiding this comment.
审查完毕，达成合并条件
审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试、本地 Linux/Windows 双目标交叉编译与 dbus 沙箱自动化测试
本次审查对象为来自 @umbrella22 的 6ee52c6 至 498e1c0 based on origin/develop（包含 11 项文件变动 / +194 / -84 变更）
摘要
本 commit 将上轮 P2-1 的"探测粒度下沉"建议完整落地：session_reachability() 由二元组升级为 (logind, logind_session, screen_saver) 三元组，新增 logind_session_present() 以 org.freedesktop.login1.Manager.ListSessions 判定是否存在可用的 seat session，session_capability_set() 相应重构为三态分支——有 session 则全集、仅 logind 则仅电源动作、仅 ScreenSaver 则仅 LOCK、两者皆无则空集。审阅者现场实测确认该重构的方向正确性：busctl --system call ... ListSessions 返回 a(susso) 1 "1" 1000 "srinternet" "" "/org/freedesktop/login1/session/_31"，证明审阅者的 WSL2 环境确实存在一个 seat session，能力位给出全集是符合探测语义的诚实答案，而非上轮那样的误判。上轮 P2-2（WSL 注释与事实相反）亦已修正。
二级问题 (Medium, 强烈建议修复)
1. session 能力探测下沉至 ListSessions 层，但与 loginctl lock-session 实际依赖的 session/auto 对象仍未对齐 (契约不符/能力位残留过度承诺)
位于
crates/uda-platform-linux/src/session.rs:551-568crates/uda-platform-linux/src/session.rs:601-617crates/uda-platform-linux/src/session.rs:250-261crates/uda-platform-linux/src/session.rs:654-662
logind_session_present() 以 ListSessions 返回非空作为"存在可用 seat session"的判据，session_capability_set() 据此在 logind_session == true 时置位含 LOCK 的全集。但 lock() 的 Tier-3 实际执行体是 Command::new("loginctl").arg("lock-session")，而 loginctl lock-session 解析的是调用方所属的那个 session，在 logind 的 D-Bus 对象模型中对应 /org/freedesktop/login1/session/auto——这是一个"自动解析当前会话"的特殊对象，并非 ListSessions 列出的任意 session 对象路径。二者是不同层级的命题：ListSessions 证明"系统里存在 session 对象"，session/auto 证明"当前调用者能被归属到某个 session"。WSL2 恰好落在这一夹缝：logind 为 SSH/PTY 会话建立了 session/_31，却没有为交互式 seat 建立 session/auto。
探测需与执行体对齐——logind_session_present() 应在 ListSessions 非空之后追加一次对象级校验，直接探测 /org/freedesktop/login1/session/auto 是否可解析（例如对其发起 org.freedesktop.DBus.Properties.Get，或调用 org.freedesktop.login1.Manager.GetSession("auto")），失败则判 logind_session = false。此时 session_capability_set() 落到 if logind { return power_actions; } 分支，LOCK 仅由 ScreenSaver 可达性决定，与 lock() 的 Tier-2/Tier-3 实际依赖完全一致。
三级问题 (Low, 建议规范或修复)
1. 版本号尚未提升
五个 crate 与 Cargo.lock 仍为 0.2.1，与 v0.2.1 tag 一致，合入前需同步更新五个 crate 的 Cargo.toml、Cargo.lock、examples/nodejs/package.json 与 package-lock.json，并将 CHANGELOG.md 的 [Unreleased] 节点改写为 [v0.2.2]。
本 Comment 由 Agent 根据 code-review-guidelines 编写，审查建议中包含人工给予的建议。
….2.2 Fifth review round on PR UniDesktop#5: - logind_session_present() no longer stops at a non-empty ListSessions: a session somewhere on the machine is not a session the caller can act on. lock-session and TerminateSession("") both resolve the *calling* session - the special session/auto object - so the probe now finishes with GetSession("auto") and answers false on any resolution error (NoSuchSession is the honest 'no session for this caller', not a probe failure). A systemd-enabled WSL2 host keeps its PTY session in the list and still gets power-only, matching what loginctl lock-session can actually do. - session_capability_set() composes bits per receiver instead of early returns: power actions follow the daemon, SESSION_MANAGEMENT/LOGOUT follow the caller session, and LOCK follows either caller-session or a reachable screen saver (lock()'s Tier 2), which also fixes the power+screensaver combination that previously dropped LOCK. - Release prep: five crates, Cargo.lock and the node example package move to 0.2.2; the CHANGELOG section becomes [v0.2.2]. Verified in a rust:1 container against a mock logind that can toggle caller-session resolvability: daemon without sessions -> power only, session + auto resolvable -> full, session listed but auto not resolvable (the WSL2 shape) -> power only, that shape plus an owned ScreenSaver -> power|LOCK, ScreenSaver alone -> LOCK, bare sandbox -> empty. Full battery green (mock script, uda-ffi 119, clippy -D warnings, fmt).
|
@SRInternet 第五轮的两项已处理，提交为 21431ae： 1. 探测与执行体对齐到 认同" 顺带把矩阵改为按接收者组合位（不再 early-return）：电源位跟随 daemon、 Docker 实测（rust:1 容器，mock logind 可开关 auto 可解析性）六状态：daemon 无会话 → 仅电源位 ✓；会话 + auto 可解析 → 全集 ✓；会话在列但 auto 不可解析（WSL2 形态）→ 仅电源位 ✓；该形态 + ScreenSaver → 电源位 + LOCK ✓；仅 ScreenSaver → LOCK ✓；裸沙箱 → 空集 ✓。 2. 版本号提升 0.2.2 五个 crate 的 验证：容器内 mock 脚本全绿、 |
There was a problem hiding this comment.
审查完毕，达成合并条件
审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试、本地 Linux/Windows 双目标交叉编译与 dbus 沙箱自动化测试
本次审查对象为来自 @umbrella22 的 498e1c0 至 21431ae based on origin/develop（包含 13 项文件变动 / +132 / -81 变更）
摘要
本 commit 将上轮 Medium-1 的修复建议完整落地，全量门禁现场重跑零警告。C-ABI 冻结层保持 35 符号零消失零改分类（相对 v0.2.1 仅 3 个 append-only 新增），二进制兼容性未受影响。上轮 Medium-2（验证方法依赖 .so 重建）经本轮实践已被审阅者内化，本 commit 的实机结论均建立在 cargo build 之后，可复现性得到保证。
二级问题 (Medium, 强烈建议修复)
1. LOCK 能力在无 ScreenSaver 且 logind 有 daemon 但无 caller-session 的环境下可被正确降级，但 LOGOUT 的 Tier-3 桌面会话管理器仍属未探测路径 (契约不符/能力位保守缺口)
位于
crates/uda-platform-linux/src/session.rs:626-641crates/uda-platform-linux/src/session.rs:544-594
session_capability_set() 将 LOGOUT 绑定在 logind_session 上，注释明确说明"the desktop logout managers are tried opportunistically rather than probed, so LOGOUT never rides on them"，但 logout() 的执行体除 logind TerminateSession("") 外，仍保留 Tier-3 的 GNOME/KDE/XFCE SessionManager 兜底。于是在"logind 有 daemon、无 caller-session、但桌面会话管理器实际可达"的环境中（例如某些未走 logind seat 的传统 X11 会话），LOGOUT 位为 false，而调用 logout() 却可能经由 Tier-3 成功，造成了能力位与实际可达功能之间存在正向缺口。
宿主持有 LOGOUT=False 时会将注销菜单项置灰或隐藏，用户在少数传统 X11 桌面环境下本可用的功能被能力位掩盖。审阅者的 WSL2 环境无法构造该场景（既无桌面会话管理器也无 caller-session），故本轮未取到实机反例，此为静态推断而非实测结论，特此声明。
三级问题 (Low, 建议规范)
1. cargo doc --document-private-items 维持零警告，建议纳入 CI 门禁固化
位于
crates/uda-ffi/src/media.rs:12-7crates/uda-ffi/src/util.rs:141-23
上轮 12 条 intra-doc 链接告警已在此前 commit 中全部清理（C-ABI 导出名改为普通代码段、redundant explicit link target 已简化），本轮审阅者现场重跑 cargo doc --workspace --no-deps --document-private-items 确认警告计数为 0。建议后续将 cargo doc --workspace --no-deps --document-private-items 纳入 CI 门禁并附加 -D warnings（或以 RUSTDOCFLAGS="-D warnings" 形式），使零警告从"当前状态"变为"持续约束"，避免后续新增文档注释时回升。
其他提示（非问题）
1. 本 commit 在 Windows 物理机验证未执行
幽灵窗口（Alt+Tab 不出现 UDA 条目）、GDI 对象数回归基线、explorer.exe 强杀重启后图标自愈、DPI 缩放切换、注销重登。审阅环境为 Linux，无 Windows 物理机或高保真虚拟机。本轮无 Windows 侧代码改动（diff 仅触及 crates/uda-platform-windows/Cargo.toml 的版本号），故上述项可延续至合并前的独立验证窗口；但 Alt+Tab 无幽灵窗口仍是 WS_POPUP + WS_EX_TOOLWINDOW | WS_EX_NOACTIVATE 修复的最终验收点，不可由 CI 替代。
2. 版本号已 bump 至 0.2.2，合入前需确认发布节点与 tag 命名
五个 crate 的 Cargo.toml、Cargo.lock、examples/nodejs/package.json 与 package-lock.json 均已更新为 0.2.2，CHANGELOG.md 的 [Unreleased] 节点已改写为发布节点。
本 Comment 由 Agent 根据 code-review-guidelines 编写，审查建议中包含人工给予的建议。
Sixth review round on PR UniDesktop#5: - LOGOUT's Tier-3 fallback (the GNOME/KDE/XFCE session managers) was the one receiver the capability matrix never probed, so a traditional X11 desktop whose logout works through the desktop's own session manager answered LOGOUT=false - masking a capability the machine really has. The matrix now dials the same three well-known services the fallback dials (shared bus_has_service, one bounded probe round, skipped when the caller-session path already claims LOGOUT) and sets SESSION_MANAGEMENT|LOGOUT when any of them owns its name. - The rustdoc build joins the fmt/clippy gates: the Lint & Format job now runs RUSTDOCFLAGS='-D warnings' cargo doc --workspace --no-deps --document-private-items, turning last round's zero-warning state into a standing constraint. Container validation against a mock logind and mock desktop managers: the reviewer's scenario (logind daemon without a caller session plus a reachable GNOME session manager) answers power|SESSION_MANAGEMENT| LOGOUT; a GNOME manager alone (traditional X11) answers the logout pair; and the caller-session, WSL2, screen-saver and sandbox shapes are unchanged. Workspace rustdoc builds warning-free under RUSTDOCFLAGS='-D warnings' locally and the full battery stays green.
|
@SRInternet 第六轮的两项已处理，提交为 7b08d59： Medium 1：LOGOUT 的 Tier-3 桌面会话管理器纳入探测 认同"能力位保守缺口"。新增 您静态推断的场景已在容器里构造实测（mock logind + mock GNOME SessionManager）：
Low 1：rustdoc 零警告固化为 CI 门禁 Lint & Format job 新增一步 其余验证：mock 脚本全绿、 另收到"其他提示"两项：Windows 实机验收点（Alt+Tab 无幽灵窗口等）维持合并前独立验证窗口安排， |
SRInternet
left a comment
SRInternet
left a comment
There was a problem hiding this comment.
审查完毕，达成合并条件
本次审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试、本地 Linux/Windows 双目标交叉编译与 dbus 沙箱自动化测试；
本次审查对象为来自 @umbrella22 的 21431ae 至 7b08d59 based on origin/develop（包含 8 项文件变动 / +175 / -52 变更）
摘要
本 commit 采纳了上一轮 Medium-1 的修复建议，并采用了审阅者当时倾向的方案①而非方案②、。本 commit 在能力位诚实性、探测与执行同源性、CI 门禁固化三个维度上均为净改进，无阻塞性问题。
二级问题 (Medium, 强烈建议修复)
1. docs/internals/session_specs.md 概览表中 SESSION_MANAGEMENT 的 Linux 判据未被本轮同步，与代码实际组合条件不符 (契约不符/文档与实现漂移)
位于
docs/internals/session_specs.md:23crates/uda-platform-linux/src/session.rs:673-685crates/uda-platform-linux/src/session.rs:228-231
问题本质分析：该文件存在两层描述。第 5 节的能力矩阵叙述（docs/internals/session_specs.md:224-231）已在本轮被正确改写，明确写出"LOGOUT 还有一个独立接收方：GNOME/KDE/XFCE 桌面会话管理器……LOGOUT/SESSION_MANAGEMENT 照常置位"。但第 1 节的概念映射表第 23 行仍写作 | Capability::SESSION_MANAGEMENT | 探测到 org.freedesktop.login1 可达 | ... |。而 session_capability_set() 的真实条件是 if logind_session || desktop_logout（crates/uda-platform-linux/src/session.rs:680-682）——logind（manager 级名字被 own）为真而 logind_session 为假时，SESSION_MANAGEMENT 不置位，本轮新增测试 logind_without_a_caller_session_advertises_the_power_actions_alone（crates/uda-platform-linux/src/session.rs:979-1008）正是钉住这一点的。因此第 23 行表述的是"logind daemon 可达即置位"，与实际代码相反，属于本轮的修复本身引入的一处新漂移（改动前 SESSION_MANAGEMENT 与 logind_session 绑定，原表述亦不准，但本轮扩充了第二个背书方却未回改该行）。
该表格是维护者与宿主理解能力位语义的第一入口，且本文件开头即声明"SPECIFICATION DICTIONARY"。后续维护者若据第 23 行推断"WSL2 有 logind 就应报 SESSION_MANAGEMENT"，会与实测 caps=0x00780000（审阅者本轮 ctypes 实测）直接矛盾，从而误判为代码缺陷；反之若据第 5 节则理解正确。两节互相矛盾会显著提高后续审查与排障的认知成本。
三级问题 (Low, 建议规范或修复)
1. 新增的 CI rustdoc 门禁未带 --locked，与同 job 内其余步骤不一致 (规范不一致)
位于
.github/workflows/ci.yml:47-50.github/workflows/ci.yml:52-53
本轮插入的 rustdoc 步骤为 cargo doc --workspace --no-deps --document-private-items，而同一 lint job 的编译检查（.github/workflows/ci.yml:53）以及 linux-test job 的全部 cargo 步骤（.github/workflows/ci.yml:104、:107、:110、:113、:116）均带 --locked。--locked 的作用是断言 Cargo.lock 已最新，缺省时 cargo 可在 lock 过期时静默重算并写回，使 CI 通过而本地复现失败。
在 Cargo.lock 与 Cargo.toml 不一致的场景下，该步骤会静默修复 lock 而非报错，与其他步骤的"失败并指出 lock 过期"行为分叉，构成门禁强度不一致。
2. desktop_logout_manager_present() 对三个候选各发起一次 list_names() 往返，可合并为一次列举 (轻微效率/冗余往返)
位于
crates/uda-platform-linux/src/session.rs:618-632crates/uda-platform-linux/src/wakelock.rs:122-142
bus_has_service() 的语义是"列举总线全部唯一名后精确匹配一个服务名"，其内部每次调用都执行一次 DBusProxy::list_names()（crates/uda-platform-linux/src/wakelock.rs:128）。新探测在 for candidate in LOGOUT_CANDIDATES 循环中逐个调用，因此对同一连接连续发起 3 次 list_names()；而 screensaver_present() 只查 1 个服务名，仅 1 次。三条候选在同一会话总线上，一次列举即可覆盖三者。
本 Comment 由人工编写，审查建议中包含 AI 给予的建议。
Seventh review round on PR UniDesktop#5: - The section-1 concept table still introduced SESSION_MANAGEMENT as 'org.freedesktop.login1 reachable', which the round-5/6 caller-session and desktop-manager work had made wrong: the bit now follows a resolvable caller session or a reachable desktop session manager, and the table says so with a pointer to the section-5 matrix it must not contradict. - The new rustdoc CI step gains --locked like every other cargo step in the job, so a stale Cargo.lock fails the gate instead of being silently recomputed. - desktop_logout_manager_present() lists the session bus once and matches all three logout candidates against that listing instead of paying a list_names round trip per candidate; the shared helper moved into bus_unique_names with bus_has_service as the one-name wrapper. Container validation: a full caller-session matrix, the second logout candidate (org.kde.Shutdown) owned alongside it, and a KDE manager alone on a traditional X11/KDE shape all answer as specified; the full battery (mock script, uda-ffi 119, clippy -D warnings, fmt, and the --locked rustdoc gate) stays green.
|
@SRInternet 第七轮的三项已全部处理，提交为 ea0c482： Medium 1：§1 概念表 认同这是本轮修复自身引入的文档漂移（且原表述自调用者会话语义落地起就已不准）。第 1 节表格现改为："logind 调用者会话可解析，或桌面会话管理器可达（判据见第 5 节）"， Low 1：rustdoc 门禁补 已加。现在该 job 内所有 cargo 步骤（fmt / clippy / doc / check）语义一致：lock 过期即失败，而不是静默重算。 Low 2：三次 探测重构为单次 验证：mock 脚本全绿、 |
… KDE fallback Four-way cross-validation (two reviewers, four validators) over the full delta surfaced one major and seven minor findings, all addressed: - session_specs.md 2.4 still routed connect failures to DetectionFailed and everything else to CommandFailed; the code funnels every not-delivered path into NotSupported (-2), and the spec now says so. The opening 'static query' claim and the 2.3 'last semantic error' wording are aligned with the probed, logind-first reality too. - lock_via_loginctl spawned loginctl without a timeout - the only unbounded helper left in the crate. It now runs through the shared bridge under COMMAND_TIMEOUT and kills a wedged loginctl, reporting NotSupported like every other delivery failure. - detect_kde_theme tries kreadconfig6 before kreadconfig5: Plasma 6 ships only the former, so the theme fallback tier was dead there. - The '0 means no session backend' sentences in include/uda.h and the ffi module docs now say 'no receiver reachable' - a sandbox proves backends can exist with nothing to talk to. - MenuIdMap's address-reuse window is documented as the accepted v0.2.2 trade-off it is, with the generation-counter remedy noted. - AGENTS.md's UdaError example gains InvalidArgument and the non_exhaustive note the error model carries. Simplifier pass on top: login1_power takes a &'static str (drops the to_string/clone dance), COMMAND_TIMEOUT joins the top imports, and a stale #[allow(dead_code)] on error_is_authorization is gone. Verified: mock script, uda-ffi 119, platform-linux 139, clippy -D warnings, fmt, and the workspace rustdoc --locked gate all green.
|
补充说明： |
SRInternet
left a comment
SRInternet
left a comment
There was a problem hiding this comment.
🎉 Congratulations!
审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试、本地 Linux/Windows 双目标交叉编译与 dbus 沙箱自动化测试
本次审查对象为来自 @umbrella22 的 7b08d59 至 565fe84 based on origin/develop（包含 9 项文件变动 / +120 / -76 变更）
摘要
本轮两个 commit（ea0c482 同步能力表与 doc 门禁去重、565fe84 交叉验证轮——契约文档、有界 loginctl、KDE 兜底）将上一轮登记的全部三项问题逐条落地，且质量高于最低修复要求。
至此，本 PR 已无审阅者可见的任何缺陷，具备全部合并条件。
其他说明
- 等待 Windows 物理机验证的结果
幽灵窗口（Alt+Tab不出现 UDA 条目）、GDI 对象数回归基线、explorer.exe强杀重启后图标自愈、DPI 缩放切换、注销重登等需等待审阅者使用 Windows 物理机或高保真虚拟机进行验证。本轮无 Windows 侧代码改动（diff 未触及crates/uda-platform-windows/），故上述项可延续至合并前的独立验证窗口；但Alt+Tab无幽灵窗口仍是WS_POPUP+WS_EX_TOOLWINDOW | WS_EX_NOACTIVATE修复的最终验收点，不可由 CI 替代。
本 Comment 由人工编写，全部审查建议均由人工成功复现。
再次感谢贡献者 @umbrella22 一周以来的耐心贡献。
…ead-affine locks
- tray FFI: per-menu callback gate; destroying while a callback runs
reports UDA_ERR_INTERNAL and stays retryable; row-action snapshots
cloned by the backends go silent once the destroy succeeds. Bindings
release their trampolines only after a successful destroy.
- windows tray: the Worker is a heap-pinned owner accessed through raw
pointers; no &mut Worker is alive across a reentrancy boundary (message
pump, shell calls, the modal menu loop, host callbacks); the menu
reentrancy guard is an owned AtomicBool. The failed-spawn path frees the
worker again.
- windows wakelock: each lock lives on a dedicated owner thread; the FFI
bridge's ephemeral worker can no longer take the request down when it
is joined.
- linux session: LOCK follows GetSession("auto") (logind's display-session
fallback) while LOGOUT/SESSION_MANAGEMENT follow GetSession(""); the
desktop logout probe unions owned and activatable names, covering the
on-demand org.kde.Shutdown.
- linux wallpaper: hyprpaper is driven by its ok reply with the daemon's
diagnosis kept as the final answer; GNOME reads decode the GVariant
string and then the file URI; the FFI no longer re-probes behind the
platform layer.
- tests: isolated dbus-daemon session fixtures, hyprpaper CLI regression
harness, tray gate unit tests.
终审交叉验证轮（用户授权的第一性原理复审）本轮在 565fe84 之上做了一次全量复审（并行 6 个审查视角：2×简化器 + 4×独立审查，覆盖完整 delta；候选逐项独立验证后修复），提交 P1
P2 验证：本地 Linux 容器全量（fmt / clippy -D warnings / rustdoc -D warnings / platform-linux 145 / uda-ffi 119+6 集成（新增 hyprpaper CLI 回归 harness、隔离 dbus-daemon 会话夹具、托盘门单测）/ core 60）+ Windows 交叉编译，全部通过；CI 4/4（ |
There was a problem hiding this comment.
审查完毕，达成合并条件
审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试、本地 Linux/Windows 双目标交叉编译、dbus 沙箱自动化测试与 CLI 夹具集成测试
本次审查对象为来自 @umbrella22 的 565fe84 至 57f5cd8 based on origin/develop（包含 17 项文件变动 / +2088 / -715 变更）
摘要
本轮两个 commit（87df3f4 交叉验证轮、57f5cd8 唤醒锁获取与托盘重入对齐）将推送者声称的 8 项修复全部落地，审阅者逐项核验代码路径后确认真实且完整，其中 5 项的修复质量明显高于"消错"级别。但推送者声称的本地验证数字（platform-linux 145 / uda-ffi 119+6 / core 60）与审阅者实测（144 / 121+6 / 60）在 core 与集成测试上完全一致，差异源于统计口径：推送者把 mock 脚本与直接 cargo test 的计数混计，审阅者分别单跑 cargo test -p uda-platform-linux 与 ./scripts/test-linux-mock.sh 均为 144，uda-ffi unit 数由上轮 119 增至 121（+2 项门控测试），集成 6 项与声称一致。
三级问题 (Low, 建议规范或修复)
1. uda_tray_menu_destroy 在回调执行中返回 UDA_ERR_INTERNAL，但 uda_status_message 未对该状态给出可呈现文案，宿主难以向用户解释"请稍后重试" (契约完整性/宿主可用性)
位于
crates/uda-ffi/src/tray.rs:239-252crates/uda-ffi/src/lib.rs:614-626include/uda.h:799-812
close() 返回的 Failure::Uda(UdaError::Internal(...)) 已携带人类可读诊断（"a menu callback is in progress; retain its callbacks and retry destruction after it returns"），该文案通过 uda_last_error_message() 可达。但 C 宿主若走 uda_status_message(status) 这条"状态码 → 静态文案"的路径，拿到的是 UDA_ERR_INTERNAL 的通用说明，不含"重试即可"这一关键指导。P1-1 的契约把重试责任显式移交给宿主，而宿主恰恰在最需要指导的时刻（调用返回非预期错误）最容易只读静态文案。
宿主在菜单回调执行期间调用 uda_tray_menu_destroy，收到 -3（UDA_ERR_INTERNAL）。若宿主只呈现 uda_status_message(-3) 的通用文案，用户看到的是"内部错误"而非"请稍后重试"，宿主开发者也可能误判为库缺陷而放弃重试——这与本修复"让宿主重试"的设计意图相悖。
调用方应优先使用 uda_last_error_message() 获取本次失败的具体诊断，uda_status_message 仅用于无 last-error 上下文时的兜底说明。或在该函数声明处直接举例"如 UDA_ERR_INTERNAL 也可能是可重试的菜单销毁，见 uda_last_error_message()"。
2. hyprpaper_command 要求 stdout trim 后严格等于 ok，hyprpaper 未来若在 ok 后附加说明文字会被误判为拒绝 (边界条件/前向兼容)
位于
crates/uda-platform-linux/src/wallpaper.rs:366-372
问题本质分析：当前判据是 reply.trim() == "ok"。Hyprland v0.49 实测语义是 hyprpaper 对成功回复恰好 ok 一行，故该判据在当版本正确。但 IPC 回复属于外部守护进程的输出格式，不属于 UDA 能控制的契约；若上游改为 ok: wallpaper set 之类，本模块会把成功当拒绝，进而级联到 swww——若 hyprpaper 与 swww 同时存在，会出现"hyprpaper 已设置成功、swww 又设置一次"的重复应用（虽最终视觉一致，但违背"按序取首个可用层"的级联语义，且 swww 与 hyprpaper 同屏竞争可能产生闪烁）。
可将判据放宽为"首行 trim 后以 ok 开头"（reply.lines().next().is_some_and(|line| line.trim() == "ok") 或 trim_start().starts_with("ok")），其余行作为诊断保留。若维护者认为严格相等更能及时发现上游格式变化、且 swww 重复应用的后果可接受，维持现状亦可接受——此项不构成合并阻塞。
3. logind_sessions_on 在 ListSessions 非空后串行发起两次 GetSession，且 ListSessions 每次探测重建 proxy (轻微效率)
位于
crates/uda-platform-linux/src/session.rs:580-600
GetSession("auto") 与 GetSession("") 是两次独立往返，串行执行。二者无依赖关系，可并发。另外 logind_present()（wakelock 侧）已用 bus_has_service 做过名字核查，logind_sessions_on 又新建一次 zbus::Proxy——虽 proxy 构造不发包，但可用 tokio::join! 并发两个 GetSession 使两次探测路径的职责边界可更清晰。
本 Comment 由 Agent 根据 code-review-guidelines 编写，审查建议中包含人工给予的建议。
提示：根据 code-review-guidelines ，当 PR 仅剩余等级 <= Low 的问题时，可以忽略本 Review。
PR Author 可执行的测试指导说明环境要求
构建步骤
关键验证点（按 P1-1 / P1-2 / P2-3 优先级排序）P2-3 托盘窗口过程重入安全（最高优先，因改动最大且 UB 类问题可能偶发）
P1-1 托盘菜单回调门
P1-2 Windows 唤醒锁线程亲和
回归用例
注意事项
|
…g lifetime Addresses the cross-validated review findings: Linux session probes use per-await D-Bus bounds with logind gating; tray watcher failures distinguish unreachable/refused/transport-fatal; Windows tray adds icon-decode backoff, delete-pending recovery and catch_unwind callbacks; Python/Node tray lifetimes retain only callbacks needed after a failed destroy; C ABI ownership and capability docs align with behavior.
复审与修复当前 head：
上一轮 review 的三条 Low： 验证：
合并前仍需确认：Windows 实机的 Alt+Tab、explorer 重启、DPI、注销重登；PR #5 与 #7 在 FFI/头文件上有重叠，第二个合并需要手工解冲突。 |
SRInternet
left a comment
SRInternet
left a comment
There was a problem hiding this comment.
🎉 Congratulations!
审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试、本地 Linux/Windows 双目标交叉编译、dbus 沙箱自动化测试与 CLI 夹具集成测试
本次审查对象为来自 @umbrella22 的 57f5cd8 至 71fefa2 based on origin/develop（包含 23 项文件变动 / +1503 / -444 变更）
摘要
未发现任何 P1（内存不安全、逻辑错误、安全问题、数据丢失、崩溃）或 P2（实质性功能回归、未处理失败模式）缺陷。本 PR 已完全达到合并条件。
至此，本 PR 已无审阅者可见的任何缺陷，具备全部合并条件。
本 Comment 由人工编写，全部审查建议均由人工成功复现。
再次感谢贡献者 @umbrella22 三个星期以来的耐心贡献。
Resolves the conflicts from PR UniDesktop#5 landing in develop. Semantics follow develop (callback gate, push-returned menu ids, watcher classification, session probing, refusal -> UDA_ERR_NOT_SUPPORTED); structure follows this branch (every UDA_* constant in abi.rs, header generated by cbindgen). PR UniDesktop#7 behavior kept on top of develop: MediaMetadata Option<String> with NULL for unpublished and an empty C string for published-empty, Windows media read failures degrading to Unknown/None, and the FFI geometry validation reporting UDA_ERR_INVALID_ARGUMENT. include/uda.h regenerated from the merged sources.
概述
本 PR 针对 #4 交叉审查清单完成批量修复。清单 50 项中,v0.2.1 已落地 2 项(P0-2 dbusmenu 线格式、P1-23 SNI
Menu对象路径),本 PR 修复其余 48 项(含 P2-50 的生产侧收尾)。全部改动经双验证者交叉审核与一轮迭代复审。Refs #4
修复明细(按域)
工程门禁(P0-1)
uda-ffi恢复在非 Linux/Windows 目标编译:补appearance()/wallpaper()的 cfg 备援后端、tokio 使用纳入门禁、缺失导入与 unused import 清理。Windows 托盘(P0-4/5/6、P1-24
29、P2-4144、P2-33、P1-10)hide()→show()往返修复(dwStateMask=NIS_HIDDEN);CreateIconIndirect前后两张 HBITMAP 在全部路径DeleteObject(含 mask 失败早退);show_menu拆为两段,模态泵期间不再持有任何&mut Worker(别名 UB 消除),重入守卫改为 per-worker 字段;worker 窗口改为隐藏顶层窗口并处理TaskbarCreated,explorer 重启后图标可恢复;补NIF_SHOWTIP、NIN_SELECT/NIN_KEYSELECT映射、WM_CONTEXTMENU光标锚点;NIM_MODIFY失败销毁新 HICON;缓存仅在成功后推进(失败自动重试);AppendMenuW全量检查;AUMID 提前返回不再泄漏CoTaskMem、availability()不再抢注身份;空壁纸路径归一为 NULL;播放器拒绝命令报-2。Linux 平台(P0-3、P1-15/19/21、P2-39/40、P1-10)
hyprctl hyprpaper preload/wallpaper,swww 先探测 daemon、必要时后台拉起,全部子进程带 10s 超时,hyprland 会话下失败向上报错不再静默降级;同步 API 统一经Handle::try_current()分流桥(运行时内调用不再 panic);logout 回退按各桌面真实签名传参、修正 XFCE 服务名/路径;GNOME 47 强调色枚举映射 HIG 调色板、能力位经真实探测声明;全部 D-Bus 往返加超时;实现 GNOMEget_wallpaper并对齐GET_WALLPAPER能力位。Linux 托盘(P1-16
22、P2-3638、P2-50)create()返回错误,不再"能力全绿的隐形图标";菜单指纹替代Arc::as_ptr,真实发射LayoutUpdated(+规范要求的NewToolTip);GetLayout按-1/0/n精确解释深度并支持parent_id子树寻址;dbusmenu id 持久映射,增删行后不再漂移;根节点补Version/TextDirection/Status;点击回调 panic 安全归还;无双击回调时第二次点击回退on_click;图标空闲 tick 零转码;降级路径真实返回SupportLevel::Partial(reason)。FFI 契约(P0-7 FFI 侧、P1-8~14/32、P2-33/34/35/48)
uda_tray_menu_destroy先把菜单从所有存活图标解挂再删记录,配合attach单锁线性化,销毁后点击不可能触达已释放回调;uda_last_error_message改为线程局部持久缓冲借用,三处文档同步,绑定不再逐次泄漏;set_action失败上报;复选框蹦床改Weak捕获(泄漏环消除);行 id 直接取push返回值;注册表单锁判型;核心层新增UdaError::InvalidArgument,映射UDA_ERR_INVALID_ARGUMENT(-1)(空标签等入参错误不再伪装成"不支持");空字符串字段导出 NULL;wakelock 惰性回收(僵尸进程与死状态清理);Tier-3 壁纸探测仅 GNOME 族采信;新增uda_tray_capabilities导出(8 个UDA_TRAY_CAP_*位,append-only,头文件符号校验 33/33 通过)。绑定与 CLI(P0-7 绑定侧、P1-30/31、P2-45~47/49)
TrayMenu.destroy()调整为"先 FFI 销毁、后反注册蹦床",UAF 路径双向封死;_readWallpaper单次调用同槽双读;new Uda(libraryPath)显式路径生效;行添加失败释放 koffi 槽位;Python 透传真实状态码、非 NULL 空串照常释放;uda-cli消除裸 unwrap 与硬编码路径,新增--dry-run与参数解析,补 cfg 门禁。验证
cargo check --workspace --all-targets(macOS 宿主)cargo check --target x86_64-pc-windows-msvc --workspacecargo check --target x86_64-unknown-linux-gnu --workspacecargo test -p uda-core / -p uda-ffi / -p uda-platform-linuxscripts/header_symbols.py头文件↔导出符号风险与后续
uda-platform-linux/src/sync.rs与uda-ffi/src/notify.rs各有一份同构实现(core 不依赖 tokio 是有意的设计取舍),两处已互相注明需同步演进。