feat(cu): add desktop presentation lifecycle - #896
Conversation
|
@astrohan This Computer Use stack slice is ready for review. Please follow the dependency and merge order stated at the top of the PR; the exact net diff is linked there. |
16d8701 to
9be7515
Compare
|
@astrohan P0 blocker found in live concurrent use: the current compatibility driver PID-bound CGEvent path does not move the pointer or steal focus, but it can still interfere with the user physical mouse button state. Do not merge the Computer Use stack as fully background-safe yet. I am preparing a fail-closed follow-up that disables every CGEvent mouse/keyboard path and retains only AX/CDP/screenshot operations until the native event backend can prove isolation. |
93b2bf9 to
e645f29
Compare
e645f29 to
3e29b92
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Approved on content. I found no P0 or P1. The branch still needs to follow its parents before merge; none of these findings requires another fix round.
P2
hasOpenWindows()counts every ElectronBrowserWindow, including the cursor overlay (apps/desktop/src/main/main-window.ts:430-432). If the main window closes while an overlay remains alive, Dock activation or a second launch takes the focus branch instead of recreating the main window (apps/desktop/src/main/main.ts:492-500). The controller then tries to focus a missing main window, leaving the app without usable UI until the overlay is torn down or the process is relaunched.clearSession()resolves presentation-ready waiters, butrunWithPresentation()does not recheck the presentation generation after that wait (packages/runtime/src/computer-use-tools.ts:978-988, 1512-1522). An action without an action lease, such aswait, can still enter backend dispatch after the clear and hold the process-wide presentation queue until it finishes. Desktop's immediate runtime abort narrows this path, so it remains P2.
P3
- Overlay cancel reports
finishedand clearsactiveActionId, but it does not stop the cursor engine's active path or click pulse (apps/desktop/src/overlay/cursor-overlay.ts:128-133). The canceled animation can continue visibly. - Runtime releases presentation ownership as soon as backend dispatch returns and does not wait for
fence.finished(packages/runtime/src/computer-use-tools.ts:965-977). A following session can start while the renderer is still completing the prior visual tail, which may be dropped. - The bloom gradient is centered on the action hotspot, but its circle is drawn around the arrow body position (
apps/desktop/src/renderer/computer-use-overlay/engine/cursor-engine.ts:152-165). The bloom is visibly offset and clipped by roughly the arrow-tip length.
The focused presentation tests passed on the reviewed patch. A restack is still required before squash merge.
3e29b92 to
fc2fd19
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Restack-only rereview on fc2fd195. Its patch-id exactly matches the reviewed presentation change. Typecheck, test, and e2e are green. No P0 or P1.
P2
hasOpenWindows()counts the cursor overlay, so Dock activation can fail to recreate a closed main window while the overlay remains.clearSession()wakes presentation readiness without a post-wait generation check; an unleased action such aswaitcan still dispatch and hold the global presentation queue.
P3
- Cancel reports the presentation finished without stopping the cursor animation.
- Presentation ownership is released before the renderer's
finishedfence, so the next session can drop the prior visual tail. - The bloom gradient and its painted circle use different centers, leaving the bloom offset by the arrow-tip length.
All remain non-blocking. Approved for squash merge with no additional fix round.
Upstream stack notice
This is stack PR F. It depends on #895 and must not merge before it.
The Files tab is cumulative until preceding fork branches are rebased after merge.
Review the exact 24-file presentation net diff now in fork-local PR hqhq1025#5.
Current rebase verification: overlay and full repository build; @maka/computer-use 109/109; Desktop typecheck; Knip overlay entry audit; upstream typecheck, test, and e2e all passed.
Goal
Add the Desktop Computer Use presentation lifecycle as a reviewable layer above verified targeting and dispatch.
What this PR does
readyForInteraction, then fails open into native dispatchfinishedbefore returning the tool resultmouse_moveagainst the bound window and returns an executor-resolved completion point without injecting native pointer inputReverse-engineering constraints applied
The implementation follows the local Codex Computer Use lab findings:
resolvedScreenPointNon-goals
Verification
npm --workspace @maka/runtime test— full suite passednpm --workspace @maka/computer-use test— 103 passednpm --workspace @maka/desktop run typechecknpm --workspace @maka/desktop test— 2419 passednpm --workspace @maka/desktop run buildgit diff --checkReal Electron smoke boundary
An earlier protocol revision produced a visible nonblank cursor screenshot. After adding session+generation acknowledgements, this desktop environment could not reliably launch a second isolated Electron app while Codex/ChatGPT Electron was active, so the current protocol's real-window smoke is intentionally deferred to the cumulative PR G harness rather than claimed as complete here.
Stacked on #4.