merge: incorporate origin/main into community-mode branch

Resolved 10 conflicted files: - VERSION/package.json: kept 0.12.0.0 (feature branch version) - CHANGELOG.md: preserved both branch entry and main's new entries - supabase/config.sh: kept GSTACK_WEB_URL, accepted TELEMETRY_ENDPOINT removal - bin/gstack-{community-dashboard,telemetry-log,telemetry-sync,update-check}: took main's improved versions (edge function approach, safe cursor, UUID gen) - supabase/functions/community-pulse: took main's count-based approach - test/telemetry.test.ts: took main's structure with fingerprint field name Post-merge fixes: - Removed shadowed local RESOLVERS/functions in gen-skill-docs.ts (main's resolver imports now take precedence for tier-based preamble, coverage gates) - Added 3 missing E2E_TIERS entries (ship-plan-*, review-plan-completion) - Updated telemetry test to match current prompt text - Regenerated all SKILL.md files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-05-20 11:19:56 +08:00 · 2026-03-24 21:01:19 -07:00
parent 56e8d20a1c 70c51d509f
commit 539a6600cd
68 changed files with 5645 additions and 495 deletions
--- a/browse/SKILL.md
+++ b/browse/SKILL.md
@@ -3,7 +3,6 @@ name: browse
 preamble-tier: 1
 version: 1.1.0
 description: |
-  MANUAL TRIGGER ONLY: invoke only when user types /browse.
  Fast headless browser for QA testing and site dogfooding. Navigate any URL, interact with
  elements, verify page state, diff before/after actions, take annotated screenshots, check
  responsive layouts, test forms and uploads, handle dialogs, and assert element states.
@@ -30,9 +29,11 @@ _SESSIONS=$(find ~/.gstack/sessions -mmin -120 -type f 2>/dev/null | wc -l | tr
 find ~/.gstack/sessions -mmin +120 -type f -delete 2>/dev/null || true
 _CONTRIB=$(~/.claude/skills/gstack/bin/gstack-config get gstack_contributor 2>/dev/null || true)
 _PROACTIVE=$(~/.claude/skills/gstack/bin/gstack-config get proactive 2>/dev/null || echo "true")
+_PROACTIVE_PROMPTED=$([ -f ~/.gstack/.proactive-prompted ] && echo "yes" || echo "no")
 _BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")
 echo "BRANCH: $_BRANCH"
 echo "PROACTIVE: $_PROACTIVE"
+echo "PROACTIVE_PROMPTED: $_PROACTIVE_PROMPTED"
 source <(~/.claude/skills/gstack/bin/gstack-repo-mode 2>/dev/null) || true
 REPO_MODE=${REPO_MODE:-unknown}
 echo "REPO_MODE: $REPO_MODE"
@@ -50,8 +51,11 @@ echo '{"skill":"browse","ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","repo":"'$(basen
 for _PF in $(find ~/.gstack/analytics -maxdepth 1 -name '.pending-*' 2>/dev/null); do [ -f "$_PF" ] && ~/.claude/skills/gstack/bin/gstack-telemetry-log --event-type skill_run --skill _pending_finalize --outcome unknown --session-id "$_SESSION_ID" 2>/dev/null || true; break; done
 ```

-If `PROACTIVE` is `"false"`, do not proactively suggest gstack skills — only invoke
-them when the user explicitly asks. The user opted out of proactive suggestions.
+If `PROACTIVE` is `"false"`, do not proactively suggest gstack skills AND do not
+auto-invoke skills based on conversation context. Only run skills the user explicitly
+types (e.g., /qa, /ship). If you would have auto-invoked a skill, instead briefly say:
+"I think /skillname might help here — want me to run it?" and wait for confirmation.
+The user opted out of proactive behavior.

 If output shows `UPGRADE_AVAILABLE <old> <new>`: read `~/.claude/skills/gstack/gstack-upgrade/SKILL.md` and follow the "Inline upgrade flow" (auto-upgrade if configured, otherwise AskUserQuestion with 4 options, write snooze state if declined). If `JUST_UPGRADED <from> <to>`: tell user "Running gstack v{to} (just updated!)" and continue.

@@ -100,6 +104,27 @@ touch ~/.gstack/.telemetry-prompted

 This only happens once. If `TEL_PROMPTED` is `yes`, skip this entirely.

+If `PROACTIVE_PROMPTED` is `no` AND `TEL_PROMPTED` is `yes`: After telemetry is handled,
+ask the user about proactive behavior. Use AskUserQuestion:
+
+> gstack can proactively figure out when you might need a skill while you work —
+> like suggesting /qa when you say "does this work?" or /investigate when you hit
+> a bug. We recommend keeping this on — it speeds up every part of your workflow.
+
+Options:
+- A) Keep it on (recommended)
+- B) Turn it off — I'll type /commands myself
+
+If A: run `~/.claude/skills/gstack/bin/gstack-config set proactive true`
+If B: run `~/.claude/skills/gstack/bin/gstack-config set proactive false`
+
+Always run:
+```bash
+touch ~/.gstack/.proactive-prompted
+```
+
+This only happens once. If `PROACTIVE_PROMPTED` is `yes`, skip this entirely.
+
 ## Contributor Mode

 If `_CONTRIB` is `true`: you are in **contributor mode**. At the end of each major workflow step, rate your gstack experience 0-10. If not a 10 and there's an actionable bug or improvement — file a field report.
--- a/browse/dist/browse
+++ b/browse/dist/browse
--- a/browse/dist/find-browse
+++ b/browse/dist/find-browse
--- a/browse/src/browser-manager.ts
+++ b/browse/src/browser-manager.ts
@@ -89,6 +89,10 @@ export class BrowserManager {

    this.browser = await chromium.launch({
      headless: useHeadless,
+      // On Windows, Chromium's sandbox fails when the server is spawned through
+      // the Bun→Node process chain (GitHub #276). Disable it — local daemon
+      // browsing user-specified URLs has marginal sandbox benefit.
+      chromiumSandbox: process.platform !== 'win32',
      ...(launchArgs.length > 0 ? { args: launchArgs } : {}),
    });

@@ -492,7 +496,11 @@ export class BrowserManager {
    // 2. Launch new headed browser (try-catch — if this fails, headless stays running)
    let newBrowser: Browser;
    try {
-      newBrowser = await chromium.launch({ headless: false, timeout: 15000 });
+      newBrowser = await chromium.launch({
+        headless: false,
+        timeout: 15000,
+        chromiumSandbox: process.platform !== 'win32',
+      });
    } catch (err: unknown) {
      const msg = err instanceof Error ? err.message : String(err);
      return `ERROR: Cannot open headed browser — ${msg}. Headless browser still running.`;
--- a/browse/src/cli.ts
+++ b/browse/src/cli.ts
@@ -76,6 +76,13 @@ export function resolveNodeServerScript(

 const NODE_SERVER_SCRIPT = IS_WINDOWS ? resolveNodeServerScript() : null;

+// On Windows, hard-fail if server-node.mjs is missing — the Bun path is known broken.
+if (IS_WINDOWS && !NODE_SERVER_SCRIPT) {
+  throw new Error(
+    'server-node.mjs not found. Run `bun run build` to generate the Windows server bundle.'
+  );
+}
+
 interface ServerState {
  pid: number;
  port: number;
@@ -96,6 +103,19 @@ function readState(): ServerState | null {
 }

 function isProcessAlive(pid: number): boolean {
+  if (IS_WINDOWS) {
+    // Bun's compiled binary can't signal Windows PIDs (always throws ESRCH).
+    // Use tasklist as a fallback. Only for one-shot calls — too slow for polling loops.
+    try {
+      const result = Bun.spawnSync(
+        ['tasklist', '/FI', `PID eq ${pid}`, '/NH', '/FO', 'CSV'],
+        { stdout: 'pipe', stderr: 'pipe', timeout: 3000 }
+      );
+      return result.stdout.toString().includes(`"${pid}"`);
+    } catch {
+      return false;
+    }
+  }
  try {
    process.kill(pid, 0);
    return true;
@@ -104,10 +124,42 @@ function isProcessAlive(pid: number): boolean {
  }
 }

+/**
+ * HTTP health check — definitive proof the server is alive and responsive.
+ * Used in all polling loops instead of isProcessAlive() (which is slow on Windows).
+ */
+export async function isServerHealthy(port: number): Promise<boolean> {
+  try {
+    const resp = await fetch(`http://127.0.0.1:${port}/health`, {
+      signal: AbortSignal.timeout(2000),
+    });
+    if (!resp.ok) return false;
+    const health = await resp.json() as any;
+    return health.status === 'healthy';
+  } catch {
+    return false;
+  }
+}
+
 // ─── Process Management ─────────────────────────────────────────
 async function killServer(pid: number): Promise<void> {
  if (!isProcessAlive(pid)) return;

+  if (IS_WINDOWS) {
+    // taskkill /T /F kills the process tree (Node + Chromium)
+    try {
+      Bun.spawnSync(
+        ['taskkill', '/PID', String(pid), '/T', '/F'],
+        { stdout: 'pipe', stderr: 'pipe', timeout: 5000 }
+      );
+    } catch {}
+    const deadline = Date.now() + 2000;
+    while (Date.now() < deadline && isProcessAlive(pid)) {
+      await Bun.sleep(100);
+    }
+    return;
+  }
+
  try { process.kill(pid, 'SIGTERM'); } catch { return; }

  // Wait up to 2s for graceful shutdown
@@ -127,6 +179,10 @@ async function killServer(pid: number): Promise<void> {
 * Verifies PID ownership before sending signals.
 */
 function cleanupLegacyState(): void {
+  // No legacy state on Windows — /tmp and `ps` don't exist, and gstack
+  // never ran on Windows before the Node.js fallback was added.
+  if (IS_WINDOWS) return;
+
  try {
    const files = fs.readdirSync('/tmp').filter(f => f.startsWith('browse-server') && f.endsWith('.json'));
    for (const file of files) {
@@ -164,44 +220,65 @@ function cleanupLegacyState(): void {
 async function startServer(): Promise<ServerState> {
  ensureStateDir(config);

-  // Clean up stale state file
+  // Clean up stale state file and error log
  try { fs.unlinkSync(config.stateFile); } catch {}
+  try { fs.unlinkSync(path.join(config.stateDir, 'browse-startup-error.log')); } catch {}

-  // Start server as detached background process.
-  // On Windows, Bun can't launch/connect to Playwright's Chromium (oven-sh/bun#4253, #9911).
-  // Fall back to running the server under Node.js with Bun API polyfills.
-  const useNode = IS_WINDOWS && NODE_SERVER_SCRIPT;
-  const serverCmd = useNode
-    ? ['node', NODE_SERVER_SCRIPT]
-    : ['bun', 'run', SERVER_SCRIPT];
-  const proc = Bun.spawn(serverCmd, {
-    stdio: ['ignore', 'pipe', 'pipe'],
-    env: { ...process.env, BROWSE_STATE_FILE: config.stateFile },
-  });
+  let proc: any = null;

-  // Don't hold the CLI open
-  proc.unref();
+  if (IS_WINDOWS && NODE_SERVER_SCRIPT) {
+    // Windows: Bun.spawn() + proc.unref() doesn't truly detach on Windows —
+    // when the CLI exits, the server dies with it. Use Node's child_process.spawn
+    // with { detached: true } instead, which is the gold standard for Windows
+    // process independence. Credit: PR #191 by @fqueiro.
+    const launcherCode =
+      `const{spawn}=require('child_process');` +
+      `spawn(process.execPath,[${JSON.stringify(NODE_SERVER_SCRIPT)}],` +
+      `{detached:true,stdio:'ignore',env:Object.assign({},process.env,` +
+      `{BROWSE_STATE_FILE:${JSON.stringify(config.stateFile)}})}).unref()`;
+    Bun.spawnSync(['node', '-e', launcherCode], { stdio: 'ignore' });
+  } else {
+    // macOS/Linux: Bun.spawn + unref works correctly
+    proc = Bun.spawn(['bun', 'run', SERVER_SCRIPT], {
+      stdio: ['ignore', 'pipe', 'pipe'],
+      env: { ...process.env, BROWSE_STATE_FILE: config.stateFile },
+    });
+    proc.unref();
+  }

-  // Wait for state file to appear
+  // Wait for server to become healthy.
+  // Use HTTP health check (not isProcessAlive) — it's fast (~instant ECONNREFUSED)
+  // and works reliably on all platforms including Windows.
  const start = Date.now();
  while (Date.now() - start < MAX_START_WAIT) {
    const state = readState();
-    if (state && isProcessAlive(state.pid)) {
+    if (state && await isServerHealthy(state.port)) {
      return state;
    }
    await Bun.sleep(100);
  }

-  // If we get here, server didn't start in time
-  // Try to read stderr for error message
-  const stderr = proc.stderr;
-  if (stderr) {
-    const reader = stderr.getReader();
+  // Server didn't start in time — try to get error details
+  if (proc?.stderr) {
+    // macOS/Linux: read stderr from the spawned process
+    const reader = proc.stderr.getReader();
    const { value } = await reader.read();
    if (value) {
      const errText = new TextDecoder().decode(value);
      throw new Error(`Server failed to start:\n${errText}`);
    }
+  } else {
+    // Windows: check startup error log (server writes errors to disk since
+    // stderr is unavailable due to stdio: 'ignore' for detachment)
+    const errorLogPath = path.join(config.stateDir, 'browse-startup-error.log');
+    try {
+      const errorLog = fs.readFileSync(errorLogPath, 'utf-8').trim();
+      if (errorLog) {
+        throw new Error(`Server failed to start:\n${errorLog}`);
+      }
+    } catch (e: any) {
+      if (e.code !== 'ENOENT') throw e;
+    }
  }
  throw new Error(`Server failed to start within ${MAX_START_WAIT / 1000}s`);
 }
@@ -237,7 +314,10 @@ function acquireServerLock(): (() => void) | null {
 async function ensureServer(): Promise<ServerState> {
  const state = readState();

-  if (state && isProcessAlive(state.pid)) {
+  // Health-check-first: HTTP is definitive proof the server is alive and responsive.
+  // This replaces the PID-gated approach which breaks on Windows (Bun's process.kill
+  // always throws ESRCH for Windows PIDs in compiled binaries).
+  if (state && await isServerHealthy(state.port)) {
    // Check for binary version mismatch (auto-restart on update)
    const currentVersion = readVersionHash();
    if (currentVersion && state.binaryVersion && currentVersion !== state.binaryVersion) {
@@ -245,21 +325,7 @@ async function ensureServer(): Promise<ServerState> {
      await killServer(state.pid);
      return startServer();
    }
-
-    // Server appears alive — do a health check
-    try {
-      const resp = await fetch(`http://127.0.0.1:${state.port}/health`, {
-        signal: AbortSignal.timeout(2000),
-      });
-      if (resp.ok) {
-        const health = await resp.json() as any;
-        if (health.status === 'healthy') {
-          return state;
-        }
-      }
-    } catch {
-      // Health check failed — server is dead or unhealthy
-    }
+    return state;
  }

  // Ensure state directory exists before lock acquisition (lock file lives there)
@@ -273,7 +339,7 @@ async function ensureServer(): Promise<ServerState> {
    const start = Date.now();
    while (Date.now() - start < MAX_START_WAIT) {
      const freshState = readState();
-      if (freshState && isProcessAlive(freshState.pid)) return freshState;
+      if (freshState && await isServerHealthy(freshState.port)) return freshState;
      await Bun.sleep(200);
    }
    throw new Error('Timed out waiting for another instance to start the server');
@@ -282,7 +348,7 @@ async function ensureServer(): Promise<ServerState> {
  try {
    // Re-read state under lock in case another process just started the server
    const freshState = readState();
-    if (freshState && isProcessAlive(freshState.pid)) {
+    if (freshState && await isServerHealthy(freshState.port)) {
      return freshState;
    }

--- a/browse/src/server.ts
+++ b/browse/src/server.ts
@@ -286,6 +286,13 @@ async function shutdown() {
 // Handle signals
 process.on('SIGTERM', shutdown);
 process.on('SIGINT', shutdown);
+// Windows: taskkill /F bypasses SIGTERM, but 'exit' fires for some shutdown paths.
+// Defense-in-depth — primary cleanup is the CLI's stale-state detection via health check.
+if (process.platform === 'win32') {
+  process.on('exit', () => {
+    try { fs.unlinkSync(config.stateFile); } catch {}
+  });
+}

 // ─── Start ─────────────────────────────────────────────────────
 async function start() {
@@ -365,5 +372,14 @@ async function start() {

 start().catch((err) => {
  console.error(`[browse] Failed to start: ${err.message}`);
+  // Write error to disk for the CLI to read — on Windows, the CLI can't capture
+  // stderr because the server is launched with detached: true, stdio: 'ignore'.
+  try {
+    const errorLogPath = path.join(config.stateDir, 'browse-startup-error.log');
+    fs.mkdirSync(config.stateDir, { recursive: true });
+    fs.writeFileSync(errorLogPath, `${new Date().toISOString()} ${err.message}\n${err.stack || ''}\n`);
+  } catch {
+    // stateDir may not exist — nothing more we can do
+  }
  process.exit(1);
 });
--- a/browse/test/config.test.ts
+++ b/browse/test/config.test.ts
@@ -248,3 +248,69 @@ describe('version mismatch detection', () => {
    expect(shouldRestart).toBe(false);
  });
 });
+
+describe('isServerHealthy', () => {
+  const { isServerHealthy } = require('../src/cli');
+  const http = require('http');
+
+  test('returns true for a healthy server', async () => {
+    const server = http.createServer((_req: any, res: any) => {
+      res.writeHead(200, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({ status: 'healthy' }));
+    });
+    await new Promise<void>(resolve => server.listen(0, resolve));
+    const port = server.address().port;
+    try {
+      expect(await isServerHealthy(port)).toBe(true);
+    } finally {
+      server.close();
+    }
+  });
+
+  test('returns false for an unhealthy server', async () => {
+    const server = http.createServer((_req: any, res: any) => {
+      res.writeHead(200, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({ status: 'unhealthy' }));
+    });
+    await new Promise<void>(resolve => server.listen(0, resolve));
+    const port = server.address().port;
+    try {
+      expect(await isServerHealthy(port)).toBe(false);
+    } finally {
+      server.close();
+    }
+  });
+
+  test('returns false when server is not running', async () => {
+    // Use a port that's almost certainly not in use
+    expect(await isServerHealthy(59999)).toBe(false);
+  });
+
+  test('returns false on non-200 response', async () => {
+    const server = http.createServer((_req: any, res: any) => {
+      res.writeHead(500);
+      res.end('Internal Server Error');
+    });
+    await new Promise<void>(resolve => server.listen(0, resolve));
+    const port = server.address().port;
+    try {
+      expect(await isServerHealthy(port)).toBe(false);
+    } finally {
+      server.close();
+    }
+  });
+});
+
+describe('startup error log', () => {
+  test('write and read error log', () => {
+    const tmpDir = path.join(os.tmpdir(), `browse-error-log-test-${Date.now()}`);
+    fs.mkdirSync(tmpDir, { recursive: true });
+    const errorLogPath = path.join(tmpDir, 'browse-startup-error.log');
+    const errorMsg = 'Cannot find module playwright';
+    fs.writeFileSync(errorLogPath, `2026-03-23T00:00:00.000Z ${errorMsg}\n`);
+    const content = fs.readFileSync(errorLogPath, 'utf-8').trim();
+    expect(content).toContain(errorMsg);
+    expect(content).toMatch(/^\d{4}-\d{2}-\d{2}T/); // ISO timestamp prefix
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+});