Skip to content

feat: add close confirmation dialogs for tabs and windows#16

Merged
Shadow-Azure merged 26 commits into
mainfrom
feat/close-confirmation-dialogs
Jun 2, 2026
Merged

feat: add close confirmation dialogs for tabs and windows#16
Shadow-Azure merged 26 commits into
mainfrom
feat/close-confirmation-dialogs

Conversation

@Shadow-Azure

@Shadow-Azure Shadow-Azure commented Jun 1, 2026

Copy link
Copy Markdown
Owner

问题描述

1. 关闭 Tab/窗口时沙箱进程残留

  • 用户关闭 Electron 窗口时,关联的沙箱进程(PTY daemon)不会被清理
  • 关闭单个 Tab 时也没有确认提示,容易误关正在运行的终端
  • 导致孤立的 sandbox-daemon 进程累积,占用系统资源

2. Electron 僵尸进程累积

  • 每次 sandbox start 都会 spawn 新的 Electron 进程
  • macOS 上 Electron 在所有窗口关闭后不会退出(window-all-closed 不触发 app.quit()
  • 新实例无法获取 requestSingleInstanceLock 后调用 app.quit(),产生僵尸进程(状态 UE,不可 kill)
  • 多次 sandbox start 后僵尸进程不断累积

3. 回归测试覆盖不足

  • release_test 发现的白屏、Tab 截图不正确等问题无法被自动化测试捕获
  • 缺少 UT 和 E2E 测试对核心功能的看护

解决方案

1. 关闭确认对话框

  • Tab 关闭: 点击 × 按钮时,如果沙箱正在运行,弹出确认对话框("终端仍在运行,确定关闭?")
  • 窗口关闭: 点击窗口关闭按钮时,弹出三选项对话框:
    • Cancel — 取消,窗口保持打开
    • Close Window Only — 仅关闭窗口,沙箱进程继续运行
    • Close All Terminals — 关闭所有终端和沙箱进程
  • IPC 协调: 主进程通过 window-closing IPC 与 renderer 通信,5 秒超时兜底
  • 使用 POST /sandbox/{id}/close 正确关闭沙箱(而非 DELETE)

2. Electron 僵尸进程修复

  • CLI 层: is_electron_running() 检查 ~/.sandbox/electron.json 中的 PID 是否存活,存活则跳过 spawn
  • Electron 层: cleanupStaleElectron() 启动时清理崩溃实例遗留的 electron.json
  • 已有实例通过 3 秒轮询自动发现新沙箱,无需重复 spawn

3. 回归测试覆盖

  • UT (vitest + jsdom):
    • connectPty.test.ts — WebSocket 延迟连接、resize 队列、消息投递(9 个测试)
    • captureToPng.test.ts — buffer fallback 深色背景检测、PNG 生成(10 个测试)
    • tabManagement.test.ts — tab 状态同步、关闭后选中逻辑(8 个测试)
  • E2E (Playwright):
    • tab-creation.spec.ts — Tab 创建、多 Tab 显示、visual regression
    • tab-screenshot.spec.ts — 终端深色主题验证、背景颜色检测
    • close-dialog.spec.ts — 关闭确认对话框完整流程(5 个测试)
  • CI 集成: Playwright E2E 作为 CI 门禁的一部分

Test plan

  • Tab 关闭确认: 点击 × → 弹出对话框 → Cancel 保持打开 → Close 正确关闭
  • 窗口关闭确认: 三选项对话框 → Close Window Only / Close All Terminals
  • 关闭一个 Tab 不影响其他 Tab
  • sandbox start 多次后仅 1 个 Electron 主进程,无新僵尸
  • 截图功能: 深色背景渲染正确,无白屏
  • 多窗口独立截图: opencode 和 zsh 截图内容独立
  • 全部 143 Rust + 32 UT + 10 E2E 测试通过
  • release.sh 构建成功 + 手动 release_test 全部 PASS

🤖 Generated with Claude Code

ZN-Ice and others added 8 commits June 1, 2026 22:13
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add onWindowClosing and sendCloseResponse to the preload bridge and
renderer type declarations to support close confirmation dialogs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…stency, double-close guard

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown

🔒 门禁检查结果

检查项 状态
Rust 格式化 ✅ success
Rust Clippy ✅ success
Rust 测试 & 覆盖率 ✅ success
前端测试 & 覆盖率 ✅ success
Playwright E2E ✅ success
统一测试 (test.sh) ✅ success
安全检查 ✅ success

Rust 测试覆盖率

指标 覆盖率
行覆盖率 69.1%
分支覆盖率 0.0%
模块 行覆盖率
crates.sandbox-core.src.sandbox ███████████████████░ 98.8%
crates.sandbox-core.src.process █████████████░░░░░░░ 65.7%
crates.sandbox-core.src.automation ███████████░░░░░░░░░ 58.8%
crates.sandbox-core.src.daemon ██████████░░░░░░░░░░ 51.8%
crates.sandbox-core.src.server █████████████████░░░ 87.5%
crates.sandbox-core.src.instance ███████████████████░ 99.1%
crates.sandbox-core.src.capture ████████████████████ 100.0%
crates.sandbox-core.src ███████████░░░░░░░░░ 55.3%

详细报告见 Rust 覆盖率 artifact

前端测试覆盖率

指标 覆盖率
行覆盖率 ████████████████░░░░ 82.3%
分支覆盖率 68.9%
函数覆盖率 80.0%
语句覆盖率 80.0%
文件 行覆盖率
src/tests/mocks/websocket.ts ██████████████████░░ 88.0%
src/tests/mocks/xterm.ts ████████████████████ 100.0%
src/renderer/api.ts ██████████████░░░░░░ 68.2%
src/renderer/tabState.ts ████████████████████ 100.0%

详细报告见前端覆盖率 artifact

✅ 所有检查通过,可以合入

点击 Squash and merge 合并此PR

ZN-Ice and others added 18 commits June 2, 2026 12:56
…y message loss

WebSocket was created immediately in connectPty, but the onOutput callback
was registered after the function returned. Messages arriving in this window
were silently discarded, causing blank screens for zsh and claude.

Defer WebSocket creation until onOutput registers the first listener via
ensureWs(). Queue resize messages for when the connection opens.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…setup

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…etection

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…idation

Add Playwright E2E tests that verify the terminal renders with a dark
theme. The first test checks the terminal container's computed background
color is not white. The second captures a full-page screenshot for visual
regression with dynamic elements masked.

Also switch the Playwright web server from `electron-vite dev --rendererOnly`
(which still spawns Electron and exits) to a standalone Vite renderer config.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…gnore

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add maxDiffPixels tolerance and updateSnapshots: "missing" so Playwright
doesn't fail when macOS-generated snapshots are compared on Linux CI.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Cross-platform screenshots (macOS vs Linux) differ by hundreds of thousands
of pixels due to font rendering — maxDiffPixels: 500 is far too strict.
Skip toHaveScreenshot comparison tests on CI while keeping functional
tests (dark background, tab count, close dialog). Keep updateSnapshots
config as safety net for future screenshot tests.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Fix find_running_daemon_returns_none_when_no_file: use
  find_running_daemon() (validates PID liveness) instead of
  read_daemon_info() (reads file regardless). Skip if real daemon
  is running since the test only covers the no-daemon case.
- Add unified-test job to CI running test.sh on Ubuntu
- Skip Rust tests on Linux CI (macOS frameworks required)
- Update gate-result to include unified-test in pass condition

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Ubuntu's /bin/sh is dash which doesn't support set -o pipefail.
The script requires bash (#!/usr/bin/env bash).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
doCloseTab was sending DELETE /sandbox/{id} which the daemon doesn't
serve (returns 404). The error was silently caught, the tab removed
from local state, then the next 3s poll re-created it from the daemon
list — making the tab reappear.

Replace the inline fetch with closeSandbox(id) from api.ts which uses
the correct POST /sandbox/{id}/close endpoint.

Add E2E test that verifies POST /sandbox/{id}/close is called and
DELETE is NOT used.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add open: false to the Vite dev server config used by Playwright's
webServer. Without this, Vite may open a browser (VS Code on some
systems) when the dev server starts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The "Close All Terminals" button in the window close dialog was using
an inline fetch with DELETE method (404 on daemon), same bug pattern
as the tab close fix. Replaced with closeSandbox() which uses the
correct POST /sandbox/{id}/close endpoint.

Added E2E test verifying the close-all flow calls the correct endpoint.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Shadow-Azure
Shadow-Azure merged commit 3ee4291 into main Jun 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant