fix(tool): clean up EXTERNAL_CDP profile dir + Chrome subprocesses on session stop

mateclaw-server/src/main/java/vip/mate/tool/browser/BrowserLauncher.java

@@ -282,14 +282,14 @@ public class BrowserLauncher {
                     ? browser.newContext()
                     : browser.contexts().get(0);
             Page page = context.pages().isEmpty() ? context.newPage() : context.pages().get(0);
-            // The temp profile dir is owned by the spawned Chrome for the lifetime of the
-            // browser; clean up when the JVM exits since we don't track per-session shutdown
-            // here. (BrowserUseTool stops the browser explicitly on stop, so the JVM hook is
-            // a backstop for crashes.)
-            registerProfileDirCleanup(userDataDir);
             long elapsed = System.currentTimeMillis() - t0;
             trace.add(Attempt.ok(Strategy.EXTERNAL_CDP, browserBin + " + connectOverCDP(" + cdpBase + ")", elapsed));
-            return Result.success(browser, context, page, true, cdpBase, Strategy.EXTERNAL_CDP, trace);
+            // Hand ownership of `proc` and `userDataDir` to the caller — the session that
+            // consumes this Result is responsible for destroyForcibly() on the process and
+            // deleteQuietly() on the dir when it stops. Spring's @PreDestroy on BrowserUseTool
+            // closes every active session on shutdown, so a clean JVM exit will not leak.
+            return Result.successOwned(browser, context, page, cdpBase, Strategy.EXTERNAL_CDP, trace,
+                    userDataDir, proc);
         } catch (Exception e) {
             proc.destroyForcibly();
             deleteQuietly(userDataDir);
@@ -299,8 +299,28 @@ public class BrowserLauncher {
         }
     }
 
-    private static void deleteQuietly(Path dir) {
+    /**
+     * Best-effort recursive delete with a single short retry. Per-file failures are
+     * swallowed so a single locked file (Windows: Chrome leaves lockfiles open briefly
+     * after exit) does not abort cleanup of the rest of the directory.
+     *
+     * <p>The retry is necessary because Chrome's GPU process and segmentation_platform
+     * DB take a few hundred milliseconds longer than the parent chrome.exe to flush
+     * and release file handles even after destroyForcibly() — without the retry,
+     * ~6 files (ShaderCache, ukm_db) are typically left on disk per session on Windows.
+     *
+     * <p>Public so {@code BrowserSession.close()} can reuse it without duplicating logic.
+     */
+    public static void deleteQuietly(Path dir) {
         if (dir == null) return;
+        deleteOnce(dir);
+        if (!Files.exists(dir)) return;
+        // Second pass: give lingering Chrome subprocesses ~500ms to release locks.
+        try { Thread.sleep(500); } catch (InterruptedException e) { Thread.currentThread().interrupt(); return; }
+        deleteOnce(dir);
+    }
+
+    private static void deleteOnce(Path dir) {
         try {
             if (!Files.exists(dir)) return;
             try (var stream = Files.walk(dir)) {
@@ -311,11 +331,6 @@ public class BrowserLauncher {
         } catch (Exception ignored) {}
     }
 
-    private static void registerProfileDirCleanup(Path dir) {
-        Runtime.getRuntime().addShutdownHook(new Thread(() -> deleteQuietly(dir),
-                "mateclaw-cdp-profile-cleanup"));
-    }
-
     // ==================== Helpers ====================
 
     private BrowserType.LaunchOptions baseLaunchOptions(boolean headed) {
@@ -551,10 +566,25 @@ public class BrowserLauncher {
         private final List<Attempt> attempts;
         private final boolean success;
         private final String failureSummary;
+        /**
+         * Temp profile directory created for {@link Strategy#EXTERNAL_CDP}.
+         * Null for every other strategy (caller does not own the user data).
+         * The session that consumes this Result is responsible for deleting the
+         * directory after it stops the browser; {@link BrowserLauncher#deleteQuietly}
+         * is provided as the canonical implementation.
+         */
+        private final Path userDataDir;
+        /**
+         * The Chrome subprocess we spawned for {@link Strategy#EXTERNAL_CDP}.
+         * Null otherwise. The session must {@code destroyForcibly()} it on stop —
+         * closing the Playwright {@code Browser} only severs the CDP connection.
+         */
+        private final Process ownedProcess;
 
         private Result(Browser browser, BrowserContext context, Page page,
                        boolean connectedViaCdp, String cdpUrl, Strategy strategy,
-                       List<Attempt> attempts, boolean success, String failureSummary) {
+                       List<Attempt> attempts, boolean success, String failureSummary,
+                       Path userDataDir, Process ownedProcess) {
             this.browser = browser;
             this.context = context;
             this.page = page;
@@ -564,15 +594,24 @@ public class BrowserLauncher {
             this.attempts = attempts;
             this.success = success;
             this.failureSummary = failureSummary;
+            this.userDataDir = userDataDir;
+            this.ownedProcess = ownedProcess;
         }
 
         static Result success(Browser browser, BrowserContext context, Page page,
                               boolean cdp, String cdpUrl, Strategy strategy, List<Attempt> attempts) {
-            return new Result(browser, context, page, cdp, cdpUrl, strategy, attempts, true, null);
+            return new Result(browser, context, page, cdp, cdpUrl, strategy, attempts, true, null, null, null);
+        }
+
+        static Result successOwned(Browser browser, BrowserContext context, Page page,
+                                   String cdpUrl, Strategy strategy, List<Attempt> attempts,
+                                   Path userDataDir, Process ownedProcess) {
+            return new Result(browser, context, page, true, cdpUrl, strategy, attempts, true, null,
+                    userDataDir, ownedProcess);
         }
 
         static Result failure(List<Attempt> attempts, String summary) {
-            return new Result(null, null, null, false, null, null, attempts, false, summary);
+            return new Result(null, null, null, false, null, null, attempts, false, summary, null, null);
         }
     }
 }

mateclaw-server/src/main/java/vip/mate/tool/builtin/BrowserUseTool.java

@@ -257,7 +257,8 @@ public class BrowserUseTool {
         }
 
         BrowserSession session = new BrowserSession(r.getBrowser(), r.getContext(), r.getPage(),
-                headed, r.isConnectedViaCdp(), r.getCdpUrl());
+                headed, r.isConnectedViaCdp(), r.getCdpUrl(),
+                r.getUserDataDir(), r.getOwnedProcess());
         sessions.put(sessionKey, session);
         scheduleIdleCheck(sessionKey);
 
@@ -297,8 +298,10 @@ public class BrowserUseTool {
             return error("Failed to connect to CDP at " + cdpUrl + ": " + r.getFailureSummary());
         }
 
+        // action=connect_cdp attaches to a user-managed Chrome — we did not spawn it,
+        // so userDataDir / ownedProcess stay null and close() will only disconnect.
         BrowserSession session = new BrowserSession(r.getBrowser(), r.getContext(), r.getPage(),
-                true, true, r.getCdpUrl());
+                true, true, r.getCdpUrl(), null, null);
         sessions.put(sessionKey, session);
         scheduleIdleCheck(sessionKey);
 
@@ -791,18 +794,33 @@ public class BrowserUseTool {
         final boolean headed;
         final boolean connectedViaCdp;
         final String cdpUrl;
+        /**
+         * Temp profile directory we created for the EXTERNAL_CDP fallback.
+         * Null when the session connected to a user-managed Chrome (CONFIG_CDP /
+         * action=connect_cdp) or used a non-CDP launch strategy.
+         */
+        final java.nio.file.Path userDataDir;
+        /**
+         * Chrome subprocess we spawned ourselves for EXTERNAL_CDP. Null otherwise.
+         * Closing the Playwright {@code Browser} only severs the CDP socket; the
+         * actual Chrome process keeps running until we destroyForcibly() it here.
+         */
+        final Process ownedProcess;
         volatile long lastActivity;
         /** 空闲看门狗定时任务（stop 时取消，避免泄漏） */
         volatile ScheduledFuture<?> idleWatchdog;
 
         BrowserSession(Browser browser, BrowserContext context, Page page,
-                        boolean headed, boolean connectedViaCdp, String cdpUrl) {
+                        boolean headed, boolean connectedViaCdp, String cdpUrl,
+                        java.nio.file.Path userDataDir, Process ownedProcess) {
             this.browser = browser;
             this.context = context;
             this.page = page;
             this.headed = headed;
             this.connectedViaCdp = connectedViaCdp;
             this.cdpUrl = cdpUrl;
+            this.userDataDir = userDataDir;
+            this.ownedProcess = ownedProcess;
             this.lastActivity = System.currentTimeMillis();
         }
 
@@ -815,15 +833,40 @@ public class BrowserUseTool {
         }
 
         /**
-         * 关闭浏览器会话（不关闭共享 Playwright）。
+         * Close the session (does not touch the shared Playwright driver).
-         * CDP 模式：仅断开连接，Chrome 进程继续运行。
+         * <ul>
-         * Launch 模式：关闭 context + browser（终止 Chromium 进程）。
+         *   <li>User-managed CDP (ownedProcess == null): just disconnect — the user owns the Chrome process.</li>
+         *   <li>Self-spawned CDP (ownedProcess != null): disconnect, then destroyForcibly() the Chrome we spawned,
+         *       wait briefly for it to exit so Windows lockfiles are released, then deleteQuietly() the temp profile.</li>
+         *   <li>Launch mode (connectedViaCdp == false): close context + browser; Playwright handles process teardown.</li>
+         * </ul>
          */
         void close() {
             if (connectedViaCdp) {
                 try {
                     if (browser != null) browser.close();
                 } catch (Exception ignored) {}
+                if (ownedProcess != null) {
+                    try {
+                        // Snapshot descendants BEFORE killing the parent. Chrome on Windows
+                        // spawns ~6 child processes (renderer, GPU, network service, ...)
+                        // that hold open handles inside the user-data-dir. destroyForcibly()
+                        // only sends TerminateProcess to the parent; killing children must
+                        // be done separately, otherwise the temp dir cannot be deleted and
+                        // the orphaned chrome.exe instances keep running.
+                        java.util.List<ProcessHandle> children = ownedProcess.descendants()
+                                .toList();
+                        ownedProcess.destroyForcibly();
+                        for (ProcessHandle h : children) {
+                            try { h.destroyForcibly(); } catch (Exception ignored) {}
+                        }
+                        ownedProcess.waitFor(5, TimeUnit.SECONDS);
+                        for (ProcessHandle h : children) {
+                            try { h.onExit().get(2, TimeUnit.SECONDS); } catch (Exception ignored) {}
+                        }
+                    } catch (Exception ignored) {}
+                    BrowserLauncher.deleteQuietly(userDataDir);
+                }
             } else {
                 try {
                     if (context != null) context.close();