Merge pull request #159 from Leonxlnx/fix/profile-sweep-owner-pid

Profile sweep with an owner file: remove only profiles whose run and Chrome are gone

.claude-plugin/marketplace.json

@@ -9,7 +9,7 @@
     {
       "name": "taste-skill",
       "description": "tasteskill v2: art direction for websites, product pages, dashboards and app screens, with a library of researched directions, real pictures from open collections and a real-browser inspector.",
-      "version": "2.1.3",
+      "version": "2.1.4",
       "source": "./",
       "skills": ["./skills/taste-skill"],
       "author": {

.codex-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "taste-skill",
-  "version": "2.1.3",
+  "version": "2.1.4",
   "description": "tasteskill v2: art direction for websites, product pages, dashboards and app screens, checked in a real browser.",
   "author": {
     "name": "leonxlnx",

.github/workflows/check-skills.yml

@@ -19,3 +19,5 @@ jobs:
           node tests/frontmatter.mjs
       - name: Scripts parse
         run: for f in skills/taste-skill/scripts/*.mjs skills/taste-skill/scripts/lib/*.mjs skills/taste-skill/assets/*.js; do node --check "$f" || exit 1; done
+      - name: Browser profile cleanup regression
+        run: node --test tests/profile-cleanup.mjs

CHANGELOG.md

@@ -4,6 +4,11 @@ All notable changes to taste-skill live here. The repo follows SemVer-ish discip
 
 ---
 
+## 2.1.4 - 2026-10-20
+
+- **Fix: one inspector run could delete the browser profile of another run that was still going** ([#157](https://github.com/Leonxlnx/taste-skill/issues/157), fixed in [#158](https://github.com/Leonxlnx/taste-skill/pull/158) by @t4sh). On macOS and Linux Chrome keeps no `lockfile`, so the sweep added in 2.1.0 judged a profile unused by its age alone, and a second run started during a long one could pull that run's profile away. #158 removed the sweep and added a regression test.
+- **Leftover profiles are cleaned up again, safely.** Every profile now carries `taste-skill-owner.json` with the machine and the process IDs of the run and its Chrome. A later run removes a profile left by a killed run only when that file names this machine and neither process is alive; a profile without the file is never touched. Profiles left by earlier versions are not removed automatically: delete the `ad-chrome-*` folders in your temp folder (or in `TASTE_SKILL_TMP`) by hand.
+
 ## 2.1.3 - 2026-10-19
 
 One repo, one skill. Nothing in `design-taste-frontend` changed.

skills/taste-skill/scripts/lib/cdp.mjs

@@ -2,8 +2,8 @@
 // Drives an installed Chrome / Edge / Chromium in headless mode over Node's built-in WebSocket.
 // Requires Node 22+. No npm install.
 import { spawn, spawnSync } from 'node:child_process';
-import { existsSync, mkdtempSync, rmSync, writeFileSync, readdirSync, statSync, openSync, closeSync } from 'node:fs';
-import { tmpdir } from 'node:os';
+import { existsSync, mkdtempSync, rmSync, writeFileSync, readFileSync, readdirSync } from 'node:fs';
+import { tmpdir, hostname } from 'node:os';
 import { join } from 'node:path';
 
 const sleep = (ms) => new Promise((r) => setTimeout(r, ms));
@@ -37,22 +37,28 @@ export function findChrome() {
   return null;
 }
 
-// Profiles from runs that were killed before close(): removed once no Chrome holds them (a running
-// Chrome keeps the profile's lockfile open) and they are a few minutes old.
-function sweepProfiles(base) {
-  try {
-    const cutoff = Date.now() - 3 * 60 * 1000;
-    for (const name of readdirSync(base)) {
-      if (!name.startsWith('ad-chrome-')) continue;
-      const p = join(base, name);
-      try {
-        if (statSync(p).mtimeMs > cutoff) continue;
-        const lock = join(p, 'lockfile');
-        if (existsSync(lock)) closeSync(openSync(lock, 'r+')); // throws while a Chrome holds it
-        rmSync(p, { recursive: true, force: true });
-      } catch {}
-    }
-  } catch {}
+// Each profile carries an owner file naming the machine and the two processes that use it (the run and
+// its Chrome). A later run removes a profile left by a killed run only when that file names this machine
+// and neither process is alive. A profile without one (made before 2.1.4, or by another tool) is never touched.
+const OWNER = 'taste-skill-owner.json';
+
+function alive(pid) {
+  if (!Number.isInteger(pid) || pid <= 0) return false;
+  try { process.kill(pid, 0); return true; } catch (e) { return e.code === 'EPERM'; }
+}
+
+export function sweepProfiles(base) {
+  let names;
+  try { names = readdirSync(base); } catch { return; }
+  for (const name of names) {
+    if (!name.startsWith('ad-chrome-')) continue;
+    const p = join(base, name);
+    try {
+      const o = JSON.parse(readFileSync(join(p, OWNER), 'utf8'));
+      if (o.host !== hostname() || !Number.isInteger(o.pid) || alive(o.pid) || alive(o.chrome)) continue;
+      rmSync(p, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 });
+    } catch {}
+  }
 }
 
 export class Browser {
@@ -64,11 +70,15 @@ export class Browser {
     if (!exe) {
       throw new Error('No Chrome/Edge/Chromium found. Install one or set CHROME_PATH to its executable.');
     }
-    // A throwaway profile per run. Its cache is capped (each run starts cold anyway), profiles left by
-    // runs that were killed are swept once they are a few hours old, and the profile goes on exit.
+    // A throwaway profile per run, its cache capped (each run starts cold anyway), removed on exit. It is
+    // claimed with an owner file before Chrome starts, so another run's sweep can tell when it was abandoned.
     const base = process.env.TASTE_SKILL_TMP || tmpdir();
     sweepProfiles(base);
     const dir = mkdtempSync(join(base, 'ad-chrome-'));
+    const claim = (chrome) => {
+      try { writeFileSync(join(dir, OWNER), JSON.stringify({ host: hostname(), pid: process.pid, chrome })); } catch {}
+    };
+    claim();
     // On exit Chrome still holds the profile's files: stop it first, then remove the folder.
     let pid = 0;
     const drop = () => {
@@ -88,6 +98,7 @@ export class Browser {
     ];
     const proc = spawn(exe, args, { stdio: ['ignore', 'ignore', 'pipe'] });
     pid = proc.pid;
+    claim(pid);
     const wsUrl = await new Promise((resolve, reject) => {
       let buf = '';
       const t = setTimeout(() => reject(new Error('Chrome did not start within 20s')), 20000);

tests/profile-cleanup.mjs

@@ -0,0 +1,100 @@
+import assert from 'node:assert/strict';
+import { closeSync, existsSync, mkdirSync, mkdtempSync, openSync, readFileSync, rmSync, utimesSync, writeFileSync } from 'node:fs';
+import { hostname, tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { spawnSync } from 'node:child_process';
+import { fileURLToPath } from 'node:url';
+import { test } from 'node:test';
+
+const OWNER = 'taste-skill-owner.json';
+// A pid that is certainly gone: a child that has already exited.
+const deadPid = () => spawnSync(process.execPath, ['-e', '']).pid;
+const profile = (base, name, owner) => {
+  const p = join(base, name);
+  mkdirSync(p);
+  writeFileSync(join(p, 'sentinel'), 'preserve');
+  if (owner !== undefined) writeFileSync(join(p, OWNER), typeof owner === 'string' ? owner : JSON.stringify(owner));
+  return p;
+};
+
+// Run in a child so Browser.launch()'s exit/signal handlers cannot leak into the test runner.
+if (process.argv[2] === '--fixture') {
+  const { Browser } = await import('../skills/taste-skill/scripts/lib/cdp.mjs');
+  globalThis.WebSocket = class {
+    constructor() { queueMicrotask(() => this.onopen?.()); }
+    send(raw) {
+      const { id } = JSON.parse(raw);
+      queueMicrotask(() => this.onmessage?.({ data: JSON.stringify({ id, result: {} }) }));
+    }
+    close() {}
+  };
+  const base = process.env.TASTE_SKILL_TMP;
+  const profiles = ['ad-chrome-open-lock', 'ad-chrome-no-lock'].map(name => profile(base, name));
+  const gone = deadPid();
+  const abandoned = profile(base, 'ad-chrome-abandoned', { host: hostname(), pid: gone, chrome: gone });
+  let lock;
+  let browser;
+  try {
+    lock = openSync(join(profiles[0], 'lockfile'), 'w+');
+    for (const p of [...profiles, abandoned]) {
+      const old = new Date(Date.now() - 10 * 60 * 1000);
+      utimesSync(p, old, old);
+    }
+    browser = await Browser.launch();
+    for (const p of profiles) {
+      assert.ok(existsSync(join(p, 'sentinel')), 'launch deleted another profile');
+    }
+    assert.equal(existsSync(abandoned), false, 'launch kept a profile whose owners are gone');
+    const owner = JSON.parse(readFileSync(join(browser.dir, OWNER), 'utf8'));
+    assert.deepEqual(owner, { host: hostname(), pid: process.pid, chrome: browser.proc.pid }, 'launch did not claim its profile');
+  } finally {
+    if (browser) await browser.close();
+    if (lock !== undefined) closeSync(lock);
+  }
+  assert.equal(existsSync(browser.dir), false, 'close left the owned profile behind');
+} else {
+  test('launch preserves older foreign profiles and close removes its own', {
+    skip: process.platform === 'win32' && 'POSIX fake browser fixture',
+  }, () => {
+    const base = mkdtempSync(join(tmpdir(), 'taste-profile-test-'));
+    try {
+      const fakeChrome = join(base, 'fake-chrome');
+      // No real browser, network, or existing user profile is accessed.
+      writeFileSync(fakeChrome, '#!/bin/sh\nprintf "DevTools listening on ws://fixture\\n" >&2\nexec tail -f /dev/null\n', { mode: 0o755 });
+      const result = spawnSync(process.execPath, [fileURLToPath(import.meta.url), '--fixture'], {
+        env: { ...process.env, CHROME_PATH: fakeChrome, TASTE_SKILL_TMP: base },
+        encoding: 'utf8',
+        timeout: 15000,
+      });
+      assert.equal(result.error, undefined, result.error?.message);
+      assert.equal(result.status, 0, result.stdout + result.stderr);
+    } finally {
+      rmSync(base, { recursive: true, force: true });
+    }
+  });
+
+  test('sweep removes a profile only when its owner file says both processes are gone', async () => {
+    const { sweepProfiles } = await import('../skills/taste-skill/scripts/lib/cdp.mjs');
+    const base = mkdtempSync(join(tmpdir(), 'taste-sweep-test-'));
+    try {
+      const gone = deadPid();
+      const here = hostname();
+      const abandoned = profile(base, 'ad-chrome-abandoned', { host: here, pid: gone, chrome: gone });
+      const kept = [
+        profile(base, 'ad-chrome-run-alive', { host: here, pid: process.pid, chrome: gone }),
+        profile(base, 'ad-chrome-chrome-alive', { host: here, pid: gone, chrome: process.pid }),
+        profile(base, 'ad-chrome-other-machine', { host: `${here}-elsewhere`, pid: gone, chrome: gone }),
+        profile(base, 'ad-chrome-no-owner'),
+        profile(base, 'ad-chrome-torn-owner', '{"host":'),
+        profile(base, 'ad-chrome-bad-pid', { host: here, pid: 'x', chrome: gone }),
+        profile(base, 'not-a-profile', { host: here, pid: gone, chrome: gone }),
+      ];
+      sweepProfiles(base);
+      assert.equal(existsSync(abandoned), false, 'sweep kept a profile whose owners are gone');
+      for (const p of kept) assert.ok(existsSync(join(p, 'sentinel')), `sweep removed ${p}`);
+      sweepProfiles(join(base, 'missing')); // a missing base is not an error
+    } finally {
+      rmSync(base, { recursive: true, force: true });
+    }
+  });
+}