fix(window): stabilize pet dragging and decouple agent integrations - #36
Conversation
Use a dedicated drag state machine with native Windows lifecycle events, and route agent activity through a normalized adapter registry with cross-agent session isolation.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds multi-agent event contracts and Codex normalization, centralizes Tauri event publication, qualifies session identity by agent, and introduces cross-platform drag lifecycle handling with native Windows support. ChangesAgent events and dragging
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant AgentSource
participant hook_server
participant agent_events
participant status
participant LiveStatusController
AgentSource->>hook_server: receive agent event
hook_server->>agent_events: publish normalized payload
agent_events->>status: emit agent-event
status->>LiveStatusController: normalize and forward enabled event
LiveStatusController->>LiveStatusController: update agent-qualified session state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src-tauri/src/lib.rs (1)
524-525: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd regression tests for the existing
AgentEventwire format.AgentEventalready applies#[serde(rename_all = "camelCase")]. Test bothget_live_eventand theagent-eventpayload.🤖 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-tauri/src/lib.rs` around lines 524 - 525, Add regression tests covering the existing AgentEvent wire format: verify get_live_event serializes fields using camelCase and verify the agent-event payload uses the same format. Reuse the current AgentEvent fixtures or construction path and assert both event retrieval and payload serialization without changing the production serialization behavior.src/pet-drag-controller.ts (1)
113-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog the position persistence failure.
Line 119 discards every
savePositionrejection. A repeated failure to persist the pet position then produces no signal. Add aconsole.warnin the catch handler, or pass an error callback throughDragEffects.🤖 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/pet-drag-controller.ts` around lines 113 - 120, Update the savePosition rejection handling in PetDragController.finish to emit a console.warn containing the persistence error instead of silently discarding it. Preserve the existing asynchronous cleanup behavior and DragEffects interface unless using its error callback is already established.src-tauri/src/window_drag/windows.rs (1)
40-43: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard against a panic crossing the window procedure.
mark_enteredandfinishrun inside a native window procedure. A panic that unwinds across theextern "system"boundary aborts the process. Both helpers handle lock errors, so the current risk is low. Wrapping the match body instd::panic::catch_unwindwould make the handler resilient to future changes in the helpers.🤖 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-tauri/src/window_drag/windows.rs` around lines 40 - 43, Wrap the native window procedure’s match body, including the WM_NCDESTROY cleanup and calls to mark_entered and finish, in std::panic::catch_unwind so panics cannot unwind across the extern "system" boundary. Preserve the existing message handling and cleanup behavior while containing any panic within the handler.src/pet-drag-controller.test.ts (1)
90-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd coverage for
reset()and the suppression boundary.Two gaps remain.
reset()is public and clears the drag animation, but no test calls it. The suppression test advances 181 ms, so it does not pin the boundary behavior at exactlyCLICK_SUPPRESSION_MS.Add a test that calls
reset()during an active drag and assertsdirectionsends withnullandstateKind()returns"idle". Add an assertion at exactly 180 ms.🤖 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/pet-drag-controller.test.ts` around lines 90 - 99, Extend the drag controller tests around the existing completed-drag suppression case to assert shouldSuppressClick() remains true at exactly 180 ms, preserving the existing post-boundary false assertion. Add coverage for the public reset() method during an active drag, verifying directions ends with null and stateKind() returns "idle".src-tauri/src/window_drag.rs (1)
41-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAllow
dead_codeon non-Windows buildsWhen non-Windows non-test targets are checked with
-D warnings,NativeDragRegistrymethods have no production callers. Add#[cfg_attr(not(target_os = "windows"), allow(dead_code))]to theimpl NativeDragRegistryblock.🤖 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-tauri/src/window_drag.rs` around lines 41 - 57, Add #[cfg_attr(not(target_os = "windows"), allow(dead_code))] to the impl NativeDragRegistry block so its methods compile without dead-code warnings on non-Windows non-test builds, while preserving existing behavior on Windows.
🤖 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 `@src-tauri/src/window_drag.rs`:
- Around line 32-58: The native exit path must recover when no enter signal
arrives. In src-tauri/src/window_drag.rs lines 32-58, update
NativeDragRegistry::finish to clear active unconditionally while returning the
id only when the session was entered, and adjust the test at lines 118-126
accordingly. In src/pet-drag-controller.ts lines 61-69, start a bounded fallback
timer when releaseRequested becomes true and completion mode is "native", and
invoke finish(id) when it expires.
In `@src/status.ts`:
- Around line 100-107: Update acceptEvent so the config and showsAgentLiveStatus
gate runs before computing or storing lastEventKey; only assign the
deduplication key and call controller.setAgentEvent after the event is eligible
for forwarding, allowing startup catch-up to process events received before
configuration loads.
---
Nitpick comments:
In `@src-tauri/src/lib.rs`:
- Around line 524-525: Add regression tests covering the existing AgentEvent
wire format: verify get_live_event serializes fields using camelCase and verify
the agent-event payload uses the same format. Reuse the current AgentEvent
fixtures or construction path and assert both event retrieval and payload
serialization without changing the production serialization behavior.
In `@src-tauri/src/window_drag.rs`:
- Around line 41-57: Add #[cfg_attr(not(target_os = "windows"),
allow(dead_code))] to the impl NativeDragRegistry block so its methods compile
without dead-code warnings on non-Windows non-test builds, while preserving
existing behavior on Windows.
In `@src-tauri/src/window_drag/windows.rs`:
- Around line 40-43: Wrap the native window procedure’s match body, including
the WM_NCDESTROY cleanup and calls to mark_entered and finish, in
std::panic::catch_unwind so panics cannot unwind across the extern "system"
boundary. Preserve the existing message handling and cleanup behavior while
containing any panic within the handler.
In `@src/pet-drag-controller.test.ts`:
- Around line 90-99: Extend the drag controller tests around the existing
completed-drag suppression case to assert shouldSuppressClick() remains true at
exactly 180 ms, preserving the existing post-boundary false assertion. Add
coverage for the public reset() method during an active drag, verifying
directions ends with null and stateKind() returns "idle".
In `@src/pet-drag-controller.ts`:
- Around line 113-120: Update the savePosition rejection handling in
PetDragController.finish to emit a console.warn containing the persistence error
instead of silently discarding it. Preserve the existing asynchronous cleanup
behavior and DragEffects interface unless using its error callback is already
established.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cb283c0-d08c-4b85-a160-3665ec98b75c
📒 Files selected for processing (23)
src-tauri/src/agent_events.rssrc-tauri/src/hook_server.rssrc-tauri/src/lib.rssrc-tauri/src/window_drag.rssrc-tauri/src/window_drag/windows.rssrc-tauri/tauri.conf.jsonsrc/agents/codex.tssrc/agents/index.tssrc/agents/registry.test.tssrc/agents/registry.tssrc/agents/types.tssrc/live-status.test.tssrc/live-status.tssrc/main.tssrc/pet-drag-controller.test.tssrc/pet-drag-controller.tssrc/reaction-controller.test.tssrc/reaction-controller.tssrc/settings.tssrc/status.tssrc/terminal-event-ledger.test.tssrc/terminal-event-ledger.tssrc/types.ts
| impl NativeDragRegistry { | ||
| fn begin(&mut self, id: u64) -> Result<(), String> { | ||
| if self.active.is_some() { | ||
| return Err("已有窗口拖动正在进行".into()); | ||
| } | ||
| self.active = Some(NativeDragSession { id, entered: false }); | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn entered(&mut self) { | ||
| if let Some(active) = self.active.as_mut() { | ||
| active.entered = true; | ||
| } | ||
| } | ||
|
|
||
| fn cancel(&mut self, id: u64) { | ||
| if self.active.is_some_and(|active| active.id == id) { | ||
| self.active = None; | ||
| } | ||
| } | ||
|
|
||
| fn finish(&mut self) -> Option<u64> { | ||
| let active = self.active.filter(|active| active.entered)?; | ||
| self.active = None; | ||
| Some(active.id) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A native drag with no completion signal strands both layers. The backend registry and the frontend controller both wait for the native completion path, and neither has a recovery route. If WM_ENTERSIZEMOVE does not arrive, WM_EXITSIZEMOVE produces no DragEnded, so the backend session stays occupied and the frontend state stays "active". Later drags then fail, and the pet keeps the drag animation.
src-tauri/src/window_drag.rs#L32-L58: make the exit path clearactiveunconditionally, and report the id only for an entered session. Update the test at lines 118-126 to match.src/pet-drag-controller.ts#L61-L69: start a bounded timer whenreleaseRequestedbecomestrueand the completion mode is"native". Callfinish(id)when the timer expires.
📍 Affects 2 files
src-tauri/src/window_drag.rs#L32-L58(this comment)src/pet-drag-controller.ts#L61-L69
🤖 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-tauri/src/window_drag.rs` around lines 32 - 58, The native exit path must
recover when no enter signal arrives. In src-tauri/src/window_drag.rs lines
32-58, update NativeDragRegistry::finish to clear active unconditionally while
returning the id only when the session was entered, and adjust the test at lines
118-126 accordingly. In src/pet-drag-controller.ts lines 61-69, start a bounded
fallback timer when releaseRequested becomes true and completion mode is
"native", and invoke finish(id) when it expires.
| function acceptEvent(payload: RawAgentEvent): void { | ||
| const event = normalizeAgentEvent(payload); | ||
| if (!event) return; | ||
| const key = agentEventKey(event); | ||
| if (key === lastEventKey) return; | ||
| lastEventKey = key; | ||
| if (config?.codex.hooksEnabled && config.codex.showLiveStatus) controller.setAgentEvent(payload); | ||
| if (config && showsAgentLiveStatus(config, event.agent)) controller.setAgentEvent(event); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix event loss when config is not yet loaded.
lastEventKey is set unconditionally, before the config and showsAgentLiveStatus gate. If an agent event arrives before config loads, the function records lastEventKey but never calls controller.setAgentEvent. The startup catch-up call to get_live_event later fetches the same event, but the dedup check on lastEventKey now discards it too. The event is lost permanently.
Move the config gate before the dedup key assignment, so the key is only recorded when the event is actually forwarded.
🐛 Proposed fix
function acceptEvent(payload: RawAgentEvent): void {
const event = normalizeAgentEvent(payload);
if (!event) return;
- const key = agentEventKey(event);
- if (key === lastEventKey) return;
- lastEventKey = key;
- if (config && showsAgentLiveStatus(config, event.agent)) controller.setAgentEvent(event);
+ if (!config || !showsAgentLiveStatus(config, event.agent)) return;
+ const key = agentEventKey(event);
+ if (key === lastEventKey) return;
+ lastEventKey = key;
+ controller.setAgentEvent(event);
}📝 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.
| function acceptEvent(payload: RawAgentEvent): void { | |
| const event = normalizeAgentEvent(payload); | |
| if (!event) return; | |
| const key = agentEventKey(event); | |
| if (key === lastEventKey) return; | |
| lastEventKey = key; | |
| if (config?.codex.hooksEnabled && config.codex.showLiveStatus) controller.setAgentEvent(payload); | |
| if (config && showsAgentLiveStatus(config, event.agent)) controller.setAgentEvent(event); | |
| } | |
| function acceptEvent(payload: RawAgentEvent): void { | |
| const event = normalizeAgentEvent(payload); | |
| if (!event) return; | |
| if (!config || !showsAgentLiveStatus(config, event.agent)) return; | |
| const key = agentEventKey(event); | |
| if (key === lastEventKey) return; | |
| lastEventKey = key; | |
| controller.setAgentEvent(event); | |
| } |
🤖 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/status.ts` around lines 100 - 107, Update acceptEvent so the config and
showsAgentLiveStatus gate runs before computing or storing lastEventKey; only
assign the deduplication key and call controller.setAgentEvent after the event
is eligible for forwarding, allowing startup catch-up to process events received
before configuration loads.
Select a complete Windows or Unix drag module at the facade and export the same API from each platform, avoiding cross-platform dead code under strict Clippy.
1 participant
背景
Windows 原生窗口拖动与 WebView 指针事件并不共享同一套可靠的结束时序。此前拖动状态散落在 main.ts 中，并依赖 pointerup、blur 和多个延迟计时器进行清理，容易产生以下问题：
同时，实时状态链路以 Codex 事件和 Codex 配置为中心。后续接入其他 Agent 时，需要同时修改事件通道、状态控制器、状态窗口和会话账本，存在明显的耦合风险。
设计与实现
独立的拖动状态机
Windows 原生生命周期
稳定的动画优先级
通用 Agent 接入层
后续接入新 Agent 时，只需要：
宠物反应、实时状态和终止事件账本不需要针对新 Agent 修改。
兼容性
测试
新增回归覆盖包括：
手动验证建议
Windows 开发环境下重点验证长时间拖动、缓慢左右反向、快速连续拖动、纵向移动，以及拖动过程中收到 Agent 事件后松开能否恢复最新宠物状态。
Summary by CodeRabbit