Skip to content

refactor(desktop): use NSH-native residency - #6723

Merged
me2seeks merged 12 commits into
rust-previewfrom
refactor/desktop-nsh-residency
Nov 10, 2026
Merged

me2seeks merged 12 commits into
rust-previewfrom
refactor/desktop-nsh-residency

Conversation

@me2seeks

@me2seeks me2seeks commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Refine the single-engine residency already merged in #6725. Current head f67c10672 is based on 0c4de9fe3, including #6740 and the configurable monochrome tray artwork from #6738.

  • Own close/tray handlers in the persistent Bootstrap, with cleanup and desktop/terminal guards. Serve the selected built-in SVG through Host assets rather than adding assets to the trusted Bootstrap bundle.
  • Prevent native pixel-terminal desktop commands and rejected secondary-window roots from exposing offscreen windows or stealing editor focus. Handle both insertion orders and clear rejected identities on deletion/reload.
  • Use explicit Ctrl+Q Quit on Windows/Linux while retaining normal close behavior, native macOS Quit, and the stopped-engine fallback. Linux secondary windows use the application-scoped Quit action; windows disappear before bounded Host detach.
  • Keep Linux reachable when registration is absent, refused or lost. Remember confirmed tray loss through deferred GTK hides; enter Quitting before hiding windows so recovery cannot resurrect shutdown. Healthy hidden reloads remain hidden.

No Quick Tools feature or second residency engine is introduced.

Verification

Check Result
Workbench JavaScript tests 108 passed
CI planning/execution tests 15 passed
Ubuntu Desktop desktop-shell tests, including all three display-backed GTK/D-Bus cases 7 passed, 0 skipped
Targeted Biome, Rust formatting, ASF headers and whitespace checks Passed

The latest review race was reproduced before the fix with real GTK and a private StatusNotifierWatcher: two close events are handled without hiding, the watcher disappears while main is visible, then the engine is allowed to issue its delayed hides. The test fails with a deferred hide stranded main after tray loss in 0.85s on the test-only commit 5e08f9bd6. The fixed test passes in 0.28s, including both visible and mapped state and explicit Quit without resurrection. No timeout, sleep, skip or weakened assertion was added.

Native verification used the existing Ubuntu Desktop checkout via SSH, Rust 1.99.1, GTK 4.22.4, Xvfb and private D-Bus sessions. Each test ran in its own process to preserve GTK thread ownership. The complete retained-engine suite also checks secondary-window Host messages, repeated reopen, healthy hidden reload, stale registration replies, pending/refused/lost registration, and no-watcher reload/exit.

Current-head Windows/MSVC product build passed. With both JS and terminal features enabled, the Windows NSH library suite passed 227 tests, with one existing ignored test; the pixel-terminal/native-window/tray/focus regression actually executed.

Windows interactive acceptance used a disposable Host Root and the public desktop launcher attached to a foreground service Host. The agent runner cannot start a detached candidate under its containing Windows job, so no detached-startup acceptance is claimed. Native Alt+F4 hid the main window without exiting its process; invoking its real Explorer tray icon restored the same window with Settings/General still selected. Artwork changed live and the selected line artwork was verified in client storage. The human owner then pressed Ctrl+Q and confirmed that the test tray icon disappeared: the shell process exited, its launcher returned 0, and Host status showed 0 sessions and 0 active calls. The separately served test Host was then stopped; the owner's original desktop was left running. Controls were observed through Computer Use and exercised through native UI Automation/keyboard input; this is not a claim of wholly manual acceptance.

Current-head hosted CI is 42785877128; local acceptance, not waiting for this run, is the owner's authorized merge criterion. macOS physical-desktop/IME acceptance, populated historical-session recovery, packaged-product acceptance and a complete cross-platform/typecheck run were not repeated. The fresh test Root had no model configured, so no model turn or composer/IME acceptance is claimed.

Public API

  • Add Linux AppView::main_close_request(&self) -> glib::Propagation so an embedding window can honor NSH close policy.
  • Preserve on_tray_lost(callback: impl Fn() + 'static) and extend its notification to confirmed initial watcher absence, current registration refusal and watcher loss during registration. Retired replies and explicit removal do not notify.

No database schema, wire contract or source-breaking signature change is introduced.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Maka assisted with implementation, diagnosis, regression tests, verification and this description. Material commits carry Generated-by: Maka.

Checklist

  • Tests cover the change and fail without it
  • Format, typecheck and the affected suites pass locally — targeted formatting, Linux compilation and behavioral suites pass; the complete cross-platform/typecheck run is not claimed.

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@me2seeks
me2seeks marked this pull request as ready for review November 9, 2026 15:36

@Astro-Han Astro-Han left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the 33-file change at e531c8c175c6bd6b05cd4ae1e724198f3b695d43 against Rust baseline 09ca38c23ae9d46214c2743e182fe018c6544429. I found one P2 issue, detailed inline: Linux can hide its last workbench without a usable restore entry.

The change replaces destruction/snapshot/recreation on close with one retained engine and window. I checked the Bootstrap-owned close/tray handlers, embedded icon access, plugin reload and removal of the old close protocol, GTK Host replacement, AppKit permission-window ownership, Win32 explicit Quit, native runner removals and dependency changes. Keeping live state avoids reconstructing arbitrary plugin presentation state on every reopen. The Linux fallback is nevertheless a separate requirement: the NSH tray implementation currently logs registration failures and does not restore main when its watcher disappears.

My local checks pass all 169 workbench JavaScript tests, including the nine desktop lifecycle cases and six reload cases. Two additional executions of the production nativeDesktop handler with controlled desktop adapters leave main hidden, plugins active and no exit request when the tray is absent or disappears. This is a controlled handler check, not a real GTK session.

I also read completed hosted logs from CI 42636733495: Ubuntu passes all six Desktop-shell tests, including retained engine/Host/secondary-window behavior; macOS passes 107 platform-library, 10 terminal-native, 22 Desktop (230.367 seconds) and two pixel tests (3.965 seconds). Both check out merge 20b02f607c0e909ecbd6c6b8ca61cc0793329785, whose parents and tree match this head merged with 09ca38c2. Those tests do not cover the missing-tray condition. The same run has since completed successfully, including Windows and all root shards. I read the Windows native log: 226 platform tests pass with one skipped, all 22 Desktop tests pass (350.319 seconds), and both pixel tests pass (1.582 seconds). Its explicit hidden-window Quit test also passes (0.391 seconds), and checkout 20b02f607c0e909ecbd6c6b8ca61cc0793329785 matches the merge identified above. These results do not exercise or resolve the missing-tray P2. Current Rust has since advanced to 12971e440e93af1501872ec8eaf6bd781360ba92 with separate pending CI.

The actual merge into 09ca38c2 succeeds. There is no database-schema change. This and #6725 are overlapping alternative implementations: merging their current heads produces conflicts in 13 files. I did not perform local native compilation, manual desktop interaction, populated historical-session recovery or packaged-app acceptance; author-reported checks remain the author's evidence.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@M4n5ter

M4n5ter commented Nov 10, 2026

Copy link
Copy Markdown
Member

@me2seeks

English

Thanks for putting this together! #6725 just landed the initial single-engine residency refactor on rust-preview (which is why this branch now shows conflicts), but your PR solves several real bugs, shortcut UX issues, and test gaps that are still present on rust-preview.

Rather than closing this PR, could you rebase it onto current rust-preview as a focused refinement combining the best of both? Specifically:

  1. Keep residency & tray ownership in Bootstrap.tsx (nativeDesktop.ts + asset:/maka/desktop-icon.png), replacing App.tsx / desktopResidency.ts:
    • Mounting residency inside replaceable App.tsx leaves closeOnPress: false sticky on the native window if App unmounts into an error boundary (making the window X button completely unresponsive), and can strand a hidden Linux window with no tray if a UI reload fails before App mounts. Owning this in Bootstrap with cleanup avoids both.
  2. Prevent terminal sessions (both cell TUI and pixel TerminalSurface) from installing an OS tray or exposing offscreen windows:
    • Keep your !isAvailable || !platform.desktop || platform.idiom === 'terminal' guard in nativeDesktop.ts.
    • In addition, pixel terminals (TerminalSurface in Kitty/Ghostty/WezTerm/Windows Terminal) currently run with idiom = Desktop and still execute Command::Desktop (SetTray, WindowRequest::Focus, etc.). Please also guard Command::Desktop on !surface.terminal.get() in nsh-platform-macos/src/system.rs and surface.terminal_active.get().is_none() in nsh-platform-windows/src/system.rs so CLI terminal runs never spawn a tray icon or surface the hidden TerminalSurface buffer window.
  3. Platform-intuitive Quit vs Close shortcuts:
    • Windows & Linux (chrome.ts): Bring in your !mac explicit Quit Maka (shortcut: 'mod+q' -> host.post('terminal.exit')) in place of !mac && { role: 'quit' }. Currently { role: 'quit' } binds Alt+F4 on non-macOS and intercepts it before DefWindowProcW, causing Alt+F4 on Windows to exit the whole app instead of hiding to the tray while Ctrl+Q does nothing.
    • macOS (chrome.ts + nsh-platform-macos/src/window.rs): Please keep { role: 'quit' } on macOS (native terminate:) and keep #6725's window.rs fallback when no engine is running, so Cmd+Q, menu Quit, and the red close button remain responsive even while the engine is stopped waiting for a Host reconnect.
    • Linux Quit (linux.rs): Keep your application-scoped app.quit (Ctrl+Q works from secondary <Window>s too) and hide/destroy the window before awaiting bounded Host detach so an empty frame does not linger on screen during detach.
  4. Preserve #6725's Linux no-tray fallback & notice cleanup while restoring your test suites:
    • Keep #6725's tray_registered() / on_tray_lost() in nsh-platform-linux/src/tray.rs (and THIRD_PARTY_NOTICES.txt / notices.json ksni removal): when tray_registered() is true, delegate close-request through main_close_request (closeOnPress / closePress), and fall back to quitting when no tray watcher accepted the icon (also ensuring a hidden reload whose tray fails to re-register does not leave the window hidden and unreachable).
    • Restore the linux-desktop CI suite (Xvfb + dbus-run-session GTK residency tests), your stronger Windows hide-then-exit test in direct2d.rs, and products/maka/workbench/tests/desktop-close.test.mjs.
简体中文

感谢提交这份重构!#6725 刚刚先一步在 rust-preview 合入了单引擎驻留的基础改造(这也是当前分支出现冲突的原因),但你这个 PR 解决了当前 rust-preview 上仍然存在的几处真实缺陷、快捷键直觉问题以及测试缺口。

与其直接关闭这个 PR,希望你能基于最新 rust-preview 做一次 rebase,把两边的优点整合起来:

  1. 将驻留与托盘所有权放在 Bootstrap.tsx(nativeDesktop.ts + asset:/maka/desktop-icon.png),替换 App.tsx / desktopResidency.ts:
    • 把驻留挂在可替换的 App.tsx 中,一旦 App 因错误边界卸载,closePress 监听被清理但原生窗口仍保留 closeOnPress: false,会导致右上角关闭按钮 X 彻底失灵;Linux 在隐藏态下若 UI 重载失败停在 Bootstrap 报错页,也会丢失托盘并让隐藏窗口失联。放在 Bootstrap 并配齐 cleanup 可以从根源避免这两个问题。
  2. 阻止终端模式(字符 TUI 与像素终端 TerminalSurface)挂载系统托盘或暴露离屏窗口:
    • 保留 nativeDesktop.ts 中的 !isAvailable || !platform.desktop || platform.idiom === 'terminal' 守卫;
    • 同时,像素终端(Kitty/Ghostty/WezTerm/Windows Terminal 下的 TerminalSurface)当前仍以 idiom = Desktop 运行并会执行 Command::Desktop(SetTray、WindowRequest::Focus 等)。请顺手在 nsh-platform-macos/src/system.rs(!surface.terminal.get())和 nsh-platform-windows/src/system.rs(surface.terminal_active.get().is_none())中屏蔽终端态的 Command::Desktop,确保命令行运行 maka 绝不会冒出系统托盘图标或把离屏渲染窗口弹到桌面上。
  3. 更符合各平台直觉的退出与关闭快捷键:
    • Windows 与 Linux(chrome.ts):采用你在非 macOS 下的显式 Quit Maka(shortcut: 'mod+q' -> host.post('terminal.exit'))替代 !mac && { role: 'quit' }。目前 { role: 'quit' } 在非 macOS 默认绑定 Alt+F4 且先于 DefWindowProcW 拦截,导致 Windows 上按 Alt+F4 直接退出整个应用而非隐藏到托盘,Ctrl+Q 反而无效。
    • macOS(chrome.ts + nsh-platform-macos/src/window.rs):请在 macOS 上保留原生 { role: 'quit' }(走 terminate:),并保留 #6725 在 window.rs 中引擎停止态的关闭兜底,确保在 Host 断开等待重连期间 Cmd+Q、菜单退出和左上角红叉依然可用。
    • Linux 退出体验(linux.rs):保留你的应用级 app.quit(焦点在二级 <Window> 时 Ctrl+Q 同样生效),并在等待有界 Host detach 前先隐藏/销毁窗口,避免屏幕上残留最多 5 秒的空白框。
  4. 保留 #6725 的 Linux 无托盘兜底与许可证清理,同时恢复完整的自动化测试:
    • 保留 #6725 在 nsh-platform-linux/src/tray.rs 中的 tray_registered() / on_tray_lost()(以及 THIRD_PARTY_NOTICES.txt / notices.json 对 ksni 的移除):当 tray_registered() 为真时通过 main_close_request 走 NSH 的 closeOnPress / closePress 流程,无可用托盘时回退为关闭即退出(同时确保隐藏态重载后若托盘未能重新注册成功,不会把窗口留在不可达的隐藏状态)。
    • 恢复 linux-desktop CI 套件(Xvfb + dbus-run-session GTK 实测)、direct2d.rs 中“先隐藏再 terminal.exit”的测试,以及 products/maka/workbench/tests/desktop-close.test.mjs。

@me2seeks
me2seeks force-pushed the refactor/desktop-nsh-residency branch from e531c8c to b5f59bb Compare November 10, 2026 03:13

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the complete 19-file change at b5f59bb2813b25606f68dd9549e1bdb499976ff0 against its base 853052bec2f7c31aaefcf0beceeb0c7ad504fbb5, separating inherited residency changes from this follow-up. The old no-tray hiding problem reported on e531c8c1 is fixed in this version: Linux closes through the no-tray exit path, and current registration refusal or watcher absence/loss restores a hidden main window. I found no further demonstrated P0–P2 issue in the inspected change.

Residency now belongs to the persistent Bootstrap rather than a replaceable contribution, with desktop/terminal guards and the existing asset reader supplying the icon. I checked owner cleanup, localized tray handlers, explicit non-Mac Quit and retained native Mac Quit, Linux close delegation and application-scoped shutdown, the generation-fenced tray callbacks, pixel-terminal desktop-command guards on both native platforms, and the added CI routes and regression tests.

Locally, all 159 existing workbench JavaScript tests pass under Node 24.18.1, four selected CI-routing tests pass, and Biome accepts nine changed JavaScript/TypeScript files. Eight controlled checks execute unchanged extracted Rust tray-registration functions against a minimal thread-local adapter; all pass. The same checks with the base's functions give six passes and two failures for refused/absent registration recovery. These extracted-function checks are not a GTK build or a real panel test.

I separately read the hosted logs for CI 42748068828. Ubuntu actually runs all seven desktop-shell tests under Xvfb/private D-Bus, including hidden reload/tray recovery and secondary-window Quit; all pass. Windows runs three desktop-shell tests, including native close → hide → terminal.exit, and 227 NSH tests with one skipped, including pixel-terminal window/tray isolation; those pass. However, the Windows job later fails desktop_steering_freezes_each_message_and_queue_is_one_shot at 72.254s (19 passes, one failure, two not run), and CLI slice 2 fails the copy/undo test at 69.592s (18 passes, one failure, 94 skipped). The unmodified 853052be base also fails that CLI test. The cause of the remaining desktop failure is not established here. Mac results are still pending, so this is not an overall successful CI result. The logs check out 3ce685651aed0736051bce66eaf45ee1309e1969, whose parents and tree match the actual base-plus-head merge.

The newer Rust baseline a75113d8b7d0cccf181a4d4214dab962e9124ac2 now has an actual content conflict in scripts/ci.test.mjs. No database-schema or wire-contract changes are introduced by this PR. I did not perform local native platform builds, manual tray/Alt+F4/IME acceptance, a populated business session, Host replacement, old-database upgrade, or packaged-product acceptance. The author's local results remain attributed to the author.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@Astro-Han Astro-Han left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the complete 19-file author change at c99b257845d6221fca622733189cce414a824f05 against bb914bde38fc3365f9d3ae666b177ea44e54188f, including a range comparison with reviewed b5f59bb2. I found no new demonstrated P0–P2 defect in the inspected code. The previous Linux no-tray fallback and registration-loss fix remain in place; the new native focus repair still needs current-head native execution evidence.

The new change addresses an additional path in which a rejected Window.open root could subsequently be inserted as a normal terminal modal. Both native mounters now retain only live rejected root identities, prevent their Insert and ModalConfig paths from acquiring modal ownership, release a root that was inserted earlier in the same batch, and clear those identities on Delete and engine reload. I checked both mount orders, the existing detached-input focus admission, ordinary Dialog reuse after Delete/reload, terminal-active and inactive Windows cases, and the expanded Mac/Windows regression sources. The Bootstrap residency owner, no-tray shutdown and watcher-generation fences remain unchanged from the preceding reviewed fix. The CI changes preserve the newer execution-recovery routing while restoring the Linux Xvfb/private-D-Bus consumer.

On Node 24.18.1 with locked, patched dependencies, all 108 current workbench JavaScript tests and all 15 CI-planning/execution tests pass. Biome accepts the nine changed JavaScript/TypeScript files. These counts are for this changed baseline; the previous head's 159-test result is not a current-head test result. I did not locally compile or execute the Mac, Windows or GTK regressions, so the author's 227-test Windows result and before/after native focus reproduction remain attributed to the author.

The original merge into bb914bde succeeded. Rust has since advanced through #6740 and #6733 to 1cf7ea313384010c3423cae34ba7c0f27ea64e34; an actual merge of this unchanged head now conflicts in products/maka/workbench/app/nativeDesktop.ts. CI 42764405990 has completed with failure. I read the original logs: Windows compiled but its Computer Use selection had 0 tests and 97 skipped, then exited with code 4 before the later desktop suites; macOS and the CLI shard failed when agent.publishCheckpoint returned agent_history_checkpoint_unavailable after a cooperative stop. The VM duplicated-prefix assertion and encrypted replay also failed. The corresponding failures occur on the previous bb914bde base, so they do not by themselves establish a new #6723 regression. The native focus regression did not receive a passing execution result in these jobs. Newly merged #6740 changes those CI and recovery paths, but its PR and landed-base runs are still queued at this check. There are no schema or wire changes in #6723.

The already merged copy-key fix in #6731 is an inherited change. The old real-Host character-terminal keyboard test was removed by #6713; its absence does not prove the copy/undo/interruption path fixed. Test-restoration PR #6736 was closed unmerged at the user’s request: its migrated fixture failed during UI initialization before reaching Ctrl+C, so it provides no end-to-end success result. The real-Host keyboard acceptance gap remains outstanding. Manual tray/Alt+F4/IME behavior, retained real business sessions and other windows, Host replacement, old-database upgrades and packaged-product acceptance remain unverified here.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Prevent the deferred close from hiding the window after tray loss

There appears to be a race between the tray availability check here and the deferred close handler. main_close_request() sends ClosePress to the engine, and the resulting hide is applied later on the GTK thread.

If the watcher disappears in between, restore_when_tray_lost() sees the main window still visible and does nothing. The queued hide can then hide it after the tray is gone, leaving a running client without a tray entry to restore the window.

Could we ensure that a deferred hide cannot leave the window inaccessible after tray loss? A regression test could delay the close handler, process watcher disappearance while the window is still visible, then let the hide arrive and verify that the window remains reachable or the application exits.

This is based on a static control-flow review; I haven't reproduced this interleaving in a native GTK session.

Keep the main-window and tray lifecycle independent of the selected root UI, with native owner cleanup and terminal guards. Serve the existing tray icon through HostAssets without adding bootstrap assets. Give non-Mac Quit an explicit Ctrl+Q exit action while preserving the native Mac Quit role.

Generated-by: Maka
Post a real WM_CLOSE, wait for the main window to become hidden, and then exercise the existing terminal.exit bridge with closeOnPress disabled.

Generated-by: Maka
Route registered-tray close requests through NSH's native main-window policy, retain the no-tray exit path, and make Ctrl+Q application scoped. Remove windows before waiting for bounded Host detach.

Notify the existing tray-loss callback for current registration refusals and initial watcher absence, without revealing healthy hidden reloads or honoring stale replies. Restore display-backed Linux CI and real GTK/HostChannel/secondary-window regressions, including delayed private D-Bus registration and recovery.

Generated-by: Maka
Reject desktop shell commands before main and secondary window routing in the native mounters. Keep desktop layout, rendering, editing and terminal clipboard transport intact. Cover hidden native windows and the Windows tray click path without source assertions or fixed sleeps.

Generated-by: Maka
Exercise the actual OpenWindow then Insert sequence. The terminal editor must retain focus when the native window is rejected, rather than having that modal root presented as a Dialog.

Generated-by: Maka
Preserve upstream's new execution recovery routing while retaining this follow-up's display-backed Linux consumer.

Generated-by: Maka
Window.open creates a Modal root and inserts it after OpenWindow. Ignoring only Desktop commands accidentally presented that root as a normal Dialog and stole terminal input focus. Retain rejected root identity for its mounted lifetime, suppress root insertion and modal configuration, and clear it on Delete or engine teardown. Exercise ordinary Dialog presentation after node deletion and reload.

Generated-by: Maka
Reuse the native OpenWindow release sequence for roots inserted before the command. AppKit releases modal focus separately from unparenting; keep rejected content out of the terminal surface as well. Cover both insertion orders on Windows.

Generated-by: Maka
Mount an auto-focused Input under each rejected Window.open root, issue its ordinary Focus command and paste into the retained main editor before deletion or reload. Exercise the same Input through the macOS JS fixture and report the Windows wait phase and input state on failure.

Generated-by: Maka
Keep confirmed tray loss armed through GTK visibility changes, and enter Quitting before hiding windows so recovery cannot resurrect shutdown. Preserve healthy hidden reload and verify two delayed closes with real GTK and private D-Bus.

Generated-by: Maka
@me2seeks
me2seeks force-pushed the refactor/desktop-nsh-residency branch from eddc70a to f67c106 Compare November 10, 2026 08:51
@me2seeks

Copy link
Copy Markdown
Contributor Author

Maka (AI agent): addressed the deferred-close/tray-loss P2 in f67c106. The GTK owner now records confirmed tray loss and also repairs subsequent hide notifications, rather than checking visibility only at loss time. A new Quitting state is entered before hiding windows, so this recovery cannot resurrect explicit Quit. A real GTK/private-D-Bus regression gates close handling, releases the watcher while main is still visible, then issues two delayed hides; it checks both visible and mapped state and Quit. Test-only 5e08f9b fails at the stranded-window assertion (0.85s); the fix passes (0.28s). All seven Ubuntu desktop-shell tests passed in isolated processes, with healthy hidden reload and no-watcher reload/exit still covered. Workbench 108/108 and CI routing 15/15 also pass. Rebased onto merged #6740 and preserved #6738 tray settings through built-in Host SVG assets. No timeout or sleep was added. Hosted CI and manual Windows/macOS acceptance are not claimed as passing.

@me2seeks

Copy link
Copy Markdown
Contributor Author

Maka (AI agent): current-head Windows acceptance completed on the owner’s machine. MSVC product build passed; Windows NSH tests with js,terminal: 227 passed, one existing ignored, including the terminal isolation/focus regression. Real Alt+F4 hides without exiting; Explorer tray invoke restores the same window with General settings retained; artwork changes and client-storage persistence were checked. The human owner pressed Ctrl+Q and confirmed tray disappearance. I independently verified that the shell exited, launcher returned 0, and Host sessions/active calls both reached zero. The temporary service Host was then stopped; the original user instance was untouched. This is isolated native-UI acceptance, not a claim of macOS/IME/model-turn or packaged-product verification. The owner explicitly authorized merge after local Windows acceptance without waiting for hosted CI.

Windows test window after tray reopen, retaining General settings and the selected line tray artwork

@me2seeks
me2seeks merged commit f3411e7 into rust-preview Nov 10, 2026
1 check passed
@me2seeks
me2seeks deleted the refactor/desktop-nsh-residency branch November 10, 2026 10:29
@M4n5ter

M4n5ter commented Nov 10, 2026

Copy link
Copy Markdown
Member

@me2seeks I’d like to rethink the tests added by this PR from scratch: if none of them existed, what is the smallest set we would add to protect the actual behavior?

The new desktop-close.test.mjs spends roughly two-thirds of the file building its own environment, including substitutes for signals, effects, cleanup, Host, and plugin loading. That is a substantial maintenance cost, and it makes it harder to distinguish product guarantees from behavior supplied by the test itself.

Rather than preserving this structure and trimming individual assertions, I would start with these behaviors:

  • Closing and reopening the main window preserves the running session and draft.
  • Missing or lost tray support, including a delayed hide after tray loss, cannot leave the application inaccessible.
  • Explicit Quit exits and cannot be undone by the recovery logic.
  • Terminal mode cannot expose desktop windows or steal editing focus.
  • Bootstrap owns the handlers across UI changes, and cleanup leaves no stale handlers.

Please rebuild the added coverage around those guarantees, using the real runtime or existing fixtures where practical. Avoid a separate mock implementation of the lifecycle machinery, and keep implementation-detail assertions only where they are necessary to catch a specific regression.

The real GTK/D-Bus test for the delayed-hide race is a good example of coverage worth retaining: it exercises the failure at the native boundary and checks that the window is actually visible and mapped.

I’d prefer fewer tests with clear failure modes and less custom infrastructure. Any unique regression covered by the current tests should still have a trustworthy replacement before its test is removed.

@me2seeks

Copy link
Copy Markdown
Contributor Author

Maka (AI agent) follow-up: #6770 replaces the coverage rather than retaining and trimming the mock environment.

  • Deletes desktop-close.test.mjs entirely (414 lines / 10 cases), including its custom reactive/lifecycle and module-loader machinery.
  • Adds three tests using real NSH execution and existing Headless facilities: close/reopen and owner cleanup across UI changes; actual Bootstrap without a root UI; platform guards and explicit Quit menu contracts. Only the business Host boundary is a fixture.
  • Keeps the real GTK/D-Bus delayed-hide/tray-loss/Quit and native terminal tests unchanged. It does not treat fixture session/work markers as an end-to-end Agent/provider session.

Verification: 27 Workbench Rust tests, 110 remaining Workbench Node tests and 15 CI-routing tests passed. Four temporary mutations independently failed at the close, cleanup, terminal-guard and Quit assertions, then returned green after restoration. The existing CI planner also selects the Root Rust suite for a fixture-only change; no new testing API or CI suite was added.

The PR documents the unresolved native-macOS and native-Windows failures from #6723's final CI run rather than removing those tests or claiming that this test-only change fixes them. #6770 is open for review and has not been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants