Merge remote-tracking branch 'origin/main' into v0.3.6-qa-upgrades

# Conflicts:
#	test/skill-e2e.test.ts

browse/SKILL.md

@@ -10,16 +10,45 @@ description: |
 allowed-tools:
   - Bash
   - Read
+  - AskUserQuestion
 
 ---
 <!-- AUTO-GENERATED from SKILL.md.tmpl — do not edit directly -->
 <!-- Regenerate: bun run gen:skill-docs -->
 
+## Update Check (run first)
+
+```bash
+_UPD=$(~/.claude/skills/gstack/bin/gstack-update-check 2>/dev/null || .claude/skills/gstack/bin/gstack-update-check 2>/dev/null || true)
+[ -n "$_UPD" ] && echo "$_UPD" || true
+```
+
+If output shows `UPGRADE_AVAILABLE <old> <new>`: read `~/.claude/skills/gstack/gstack-upgrade/SKILL.md` and follow the "Inline upgrade flow" (AskUserQuestion → upgrade if yes, `touch ~/.gstack/last-update-check` if no). If `JUST_UPGRADED <from> <to>`: tell user "Running gstack v{to} (just updated!)" and continue.
+
 # browse: QA Testing & Dogfooding
 
 Persistent headless Chromium. First call auto-starts (~3s), then ~100ms per command.
 State persists between calls (cookies, tabs, login sessions).
 
+## SETUP (run this check BEFORE any browse command)
+
+```bash
+_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
+B=""
+[ -n "$_ROOT" ] && [ -x "$_ROOT/.claude/skills/gstack/browse/dist/browse" ] && B="$_ROOT/.claude/skills/gstack/browse/dist/browse"
+[ -z "$B" ] && B=~/.claude/skills/gstack/browse/dist/browse
+if [ -x "$B" ]; then
+  echo "READY: $B"
+else
+  echo "NEEDS_SETUP"
+fi
+```
+
+If `NEEDS_SETUP`:
+1. Tell the user: "gstack browse needs a one-time build (~10 seconds). OK to proceed?" Then STOP and wait.
+2. Run: `cd <SKILL_DIR> && ./setup`
+3. If `bun` is not installed: `curl -fsSL https://bun.sh/install | bash`
+
 ## Core QA Patterns
 
 ### 1. Verify a page loads correctly

browse/SKILL.md.tmpl

@@ -10,14 +10,19 @@ description: |
 allowed-tools:
   - Bash
   - Read
+  - AskUserQuestion
 
 ---
 
+{{UPDATE_CHECK}}
+
 # browse: QA Testing & Dogfooding
 
 Persistent headless Chromium. First call auto-starts (~3s), then ~100ms per command.
 State persists between calls (cookies, tabs, login sessions).
 
+{{BROWSE_SETUP}}
+
 ## Core QA Patterns
 
 ### 1. Verify a page loads correctly

browse/bin/find-browse

@@ -1,11 +1,11 @@
 #!/bin/bash
 # Shim: delegates to compiled find-browse binary, falls back to basic discovery.
-# The compiled binary adds version checking and META signal support.
+# The compiled binary handles git root detection for workspace-local installs.
 DIR="$(cd "$(dirname "$0")/.." && pwd)/dist"
 if test -x "$DIR/find-browse"; then
   exec "$DIR/find-browse" "$@"
 fi
-# Fallback: basic discovery (no version check)
+# Fallback: basic discovery
 ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
 if [ -n "$ROOT" ] && test -x "$ROOT/.claude/skills/gstack/browse/dist/browse"; then
   echo "$ROOT/.claude/skills/gstack/browse/dist/browse"

browse/src/find-browse.ts

@@ -1,28 +1,14 @@
 /**
- * find-browse — locate the gstack browse binary + check for updates.
+ * find-browse — locate the gstack browse binary.
  *
  * Compiled to browse/dist/find-browse (standalone binary, no bun runtime needed).
- *
- * Output protocol:
- *   Line 1: /path/to/binary              (always present)
- *   Line 2+: META:<TYPE> <json-payload>   (optional, 0 or more)
- *
- * META types:
- *   META:UPDATE_AVAILABLE — local binary is behind origin/main
- *
- * All version checks are best-effort: network failures, missing files, and
- * cache errors degrade gracefully to outputting only the binary path.
+ * Outputs the absolute path to the browse binary on stdout, or exits 1 if not found.
  */
 
 import { existsSync } from 'fs';
-import { readFileSync, writeFileSync } from 'fs';
-import { join, dirname } from 'path';
+import { join } from 'path';
 import { homedir } from 'os';
 
-const REPO_URL = 'https://github.com/garrytan/gstack.git';
-const CACHE_PATH = '/tmp/gstack-latest-version';
-const CACHE_TTL = 14400; // 4 hours in seconds
-
 // ─── Binary Discovery ───────────────────────────────────────────
 
 function getGitRoot(): string | null {
@@ -55,114 +41,6 @@ export function locateBinary(): string | null {
   return null;
 }
 
-// ─── Version Check ──────────────────────────────────────────────
-
-interface CacheEntry {
-  sha: string;
-  timestamp: number;
-}
-
-function readCache(): CacheEntry | null {
-  try {
-    const content = readFileSync(CACHE_PATH, 'utf-8').trim();
-    const parts = content.split(/\s+/);
-    if (parts.length < 2) return null;
-    const sha = parts[0];
-    const timestamp = parseInt(parts[1], 10);
-    if (!sha || isNaN(timestamp)) return null;
-    // Validate SHA is hex
-    if (!/^[0-9a-f]{40}$/i.test(sha)) return null;
-    return { sha, timestamp };
-  } catch {
-    return null;
-  }
-}
-
-function writeCache(sha: string, timestamp: number): void {
-  try {
-    writeFileSync(CACHE_PATH, `${sha} ${timestamp}\n`);
-  } catch {
-    // Cache write failure is non-fatal
-  }
-}
-
-function fetchRemoteSHA(): string | null {
-  try {
-    const proc = Bun.spawnSync(['git', 'ls-remote', REPO_URL, 'refs/heads/main'], {
-      stdout: 'pipe',
-      stderr: 'pipe',
-      timeout: 10_000, // 10s timeout
-    });
-    if (proc.exitCode !== 0) return null;
-    const output = proc.stdout.toString().trim();
-    const sha = output.split(/\s+/)[0];
-    if (!sha || !/^[0-9a-f]{40}$/i.test(sha)) return null;
-    return sha;
-  } catch {
-    return null;
-  }
-}
-
-function resolveSkillDir(binaryPath: string): string | null {
-  const home = homedir();
-  const globalPrefix = join(home, '.claude', 'skills', 'gstack');
-  if (binaryPath.startsWith(globalPrefix)) return globalPrefix;
-
-  // Workspace-local: binary is at $ROOT/.claude/skills/gstack/browse/dist/browse
-  // Skill dir is $ROOT/.claude/skills/gstack
-  const parts = binaryPath.split('/.claude/skills/gstack/');
-  if (parts.length === 2) return parts[0] + '/.claude/skills/gstack';
-
-  return null;
-}
-
-export function checkVersion(binaryDir: string): string | null {
-  // Read local version
-  const versionFile = join(binaryDir, '.version');
-  let localSHA: string;
-  try {
-    localSHA = readFileSync(versionFile, 'utf-8').trim();
-  } catch {
-    return null; // No .version file → skip check
-  }
-  if (!localSHA) return null;
-
-  const now = Math.floor(Date.now() / 1000);
-
-  // Check cache
-  let remoteSHA: string | null = null;
-  const cache = readCache();
-  if (cache && (now - cache.timestamp) < CACHE_TTL) {
-    remoteSHA = cache.sha;
-  }
-
-  // Fetch from remote if cache miss
-  if (!remoteSHA) {
-    remoteSHA = fetchRemoteSHA();
-    if (remoteSHA) {
-      writeCache(remoteSHA, now);
-    }
-  }
-
-  if (!remoteSHA) return null; // Offline or error → skip check
-
-  // Compare
-  if (localSHA === remoteSHA) return null; // Up to date
-
-  // Determine skill directory for update command
-  const binaryPath = join(binaryDir, 'browse');
-  const skillDir = resolveSkillDir(binaryPath);
-  if (!skillDir) return null;
-
-  const payload = JSON.stringify({
-    current: localSHA.slice(0, 8),
-    latest: remoteSHA.slice(0, 8),
-    command: `cd ${skillDir} && git stash && git fetch origin && git reset --hard origin/main && ./setup`,
-  });
-
-  return `META:UPDATE_AVAILABLE ${payload}`;
-}
-
 // ─── Main ───────────────────────────────────────────────────────
 
 function main() {
@@ -173,9 +51,6 @@ function main() {
   }
 
   console.log(bin);
-
-  const meta = checkVersion(dirname(bin));
-  if (meta) console.log(meta);
 }
 
 main();

browse/test/find-browse.test.ts

@@ -1,130 +1,10 @@
 /**
- * Tests for find-browse version check logic
- *
- * Tests the checkVersion() and locateBinary() functions directly.
- * Uses temp directories with mock .version files and cache files.
+ * Tests for find-browse binary locator.
  */
 
-import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
-import { checkVersion, locateBinary } from '../src/find-browse';
-import { mkdtempSync, writeFileSync, rmSync, existsSync, mkdirSync } from 'fs';
-import { join } from 'path';
-import { tmpdir } from 'os';
-
-let tempDir: string;
-
-beforeEach(() => {
-  tempDir = mkdtempSync(join(tmpdir(), 'find-browse-test-'));
-});
-
-afterEach(() => {
-  rmSync(tempDir, { recursive: true, force: true });
-  // Clean up test cache
-  try { rmSync('/tmp/gstack-latest-version'); } catch {}
-});
-
-describe('checkVersion', () => {
-  test('returns null when .version file is missing', () => {
-    const result = checkVersion(tempDir);
-    expect(result).toBeNull();
-  });
-
-  test('returns null when .version file is empty', () => {
-    writeFileSync(join(tempDir, '.version'), '');
-    const result = checkVersion(tempDir);
-    expect(result).toBeNull();
-  });
-
-  test('returns null when .version has only whitespace', () => {
-    writeFileSync(join(tempDir, '.version'), '  \n');
-    const result = checkVersion(tempDir);
-    expect(result).toBeNull();
-  });
-
-  test('returns null when local SHA matches remote (cache hit)', () => {
-    const sha = 'a'.repeat(40);
-    writeFileSync(join(tempDir, '.version'), sha);
-    // Write cache with same SHA, recent timestamp
-    const now = Math.floor(Date.now() / 1000);
-    writeFileSync('/tmp/gstack-latest-version', `${sha} ${now}\n`);
-
-    const result = checkVersion(tempDir);
-    expect(result).toBeNull();
-  });
-
-  test('returns META:UPDATE_AVAILABLE when SHAs differ (cache hit)', () => {
-    const localSha = 'a'.repeat(40);
-    const remoteSha = 'b'.repeat(40);
-    writeFileSync(join(tempDir, '.version'), localSha);
-    // Create a fake browse binary path so resolveSkillDir works
-    const browsePath = join(tempDir, 'browse');
-    writeFileSync(browsePath, '');
-    // Write cache with different SHA, recent timestamp
-    const now = Math.floor(Date.now() / 1000);
-    writeFileSync('/tmp/gstack-latest-version', `${remoteSha} ${now}\n`);
-
-    const result = checkVersion(tempDir);
-    // Result may be null if resolveSkillDir can't determine skill dir from temp path
-    // That's expected — the META signal requires a known skill dir path
-    if (result !== null) {
-      expect(result).toStartWith('META:UPDATE_AVAILABLE');
-      const jsonStr = result.replace('META:UPDATE_AVAILABLE ', '');
-      const payload = JSON.parse(jsonStr);
-      expect(payload.current).toBe('a'.repeat(8));
-      expect(payload.latest).toBe('b'.repeat(8));
-      expect(payload.command).toContain('git stash');
-      expect(payload.command).toContain('git reset --hard origin/main');
-      expect(payload.command).toContain('./setup');
-    }
-  });
-
-  test('uses cached SHA when cache is fresh (< 4hr)', () => {
-    const localSha = 'a'.repeat(40);
-    const remoteSha = 'a'.repeat(40);
-    writeFileSync(join(tempDir, '.version'), localSha);
-    // Cache is 1 hour old — should still be valid
-    const oneHourAgo = Math.floor(Date.now() / 1000) - 3600;
-    writeFileSync('/tmp/gstack-latest-version', `${remoteSha} ${oneHourAgo}\n`);
-
-    const result = checkVersion(tempDir);
-    expect(result).toBeNull(); // SHAs match
-  });
-
-  test('treats expired cache as stale', () => {
-    const localSha = 'a'.repeat(40);
-    writeFileSync(join(tempDir, '.version'), localSha);
-    // Cache is 5 hours old — should be stale
-    const fiveHoursAgo = Math.floor(Date.now() / 1000) - 18000;
-    writeFileSync('/tmp/gstack-latest-version', `${'b'.repeat(40)} ${fiveHoursAgo}\n`);
-
-    // This will try git ls-remote which may fail in test env — that's OK
-    // The important thing is it doesn't use the stale cache value
-    const result = checkVersion(tempDir);
-    // Result depends on whether git ls-remote succeeds in test environment
-    // If offline, returns null (graceful degradation)
-    expect(result === null || typeof result === 'string').toBe(true);
-  });
-
-  test('handles corrupt cache file gracefully', () => {
-    const localSha = 'a'.repeat(40);
-    writeFileSync(join(tempDir, '.version'), localSha);
-    writeFileSync('/tmp/gstack-latest-version', 'garbage data here');
-
-    // Should not throw, should treat as stale
-    const result = checkVersion(tempDir);
-    expect(result === null || typeof result === 'string').toBe(true);
-  });
-
-  test('handles cache with invalid SHA gracefully', () => {
-    const localSha = 'a'.repeat(40);
-    writeFileSync(join(tempDir, '.version'), localSha);
-    writeFileSync('/tmp/gstack-latest-version', `not-a-sha ${Math.floor(Date.now() / 1000)}\n`);
-
-    // Invalid SHA should be treated as no cache
-    const result = checkVersion(tempDir);
-    expect(result === null || typeof result === 'string').toBe(true);
-  });
-});
+import { describe, test, expect } from 'bun:test';
+import { locateBinary } from '../src/find-browse';
+import { existsSync } from 'fs';
 
 describe('locateBinary', () => {
   test('returns null when no binary exists at known paths', () => {

browse/test/gstack-update-check.test.ts

@@ -0,0 +1,188 @@
+/**
+ * Tests for bin/gstack-update-check bash script.
+ *
+ * Uses Bun.spawnSync to invoke the script with temp dirs and
+ * GSTACK_DIR / GSTACK_STATE_DIR / GSTACK_REMOTE_URL env overrides
+ * for full isolation.
+ */
+
+import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
+import { mkdtempSync, writeFileSync, rmSync, existsSync, readFileSync } from 'fs';
+import { join } from 'path';
+import { tmpdir } from 'os';
+
+const SCRIPT = join(import.meta.dir, '..', '..', 'bin', 'gstack-update-check');
+
+let gstackDir: string;
+let stateDir: string;
+
+function run(extraEnv: Record<string, string> = {}) {
+  const result = Bun.spawnSync(['bash', SCRIPT], {
+    env: {
+      ...process.env,
+      GSTACK_DIR: gstackDir,
+      GSTACK_STATE_DIR: stateDir,
+      GSTACK_REMOTE_URL: `file://${join(gstackDir, 'REMOTE_VERSION')}`,
+      ...extraEnv,
+    },
+    stdout: 'pipe',
+    stderr: 'pipe',
+  });
+  return {
+    exitCode: result.exitCode,
+    stdout: result.stdout.toString().trim(),
+    stderr: result.stderr.toString().trim(),
+  };
+}
+
+beforeEach(() => {
+  gstackDir = mkdtempSync(join(tmpdir(), 'gstack-upd-test-'));
+  stateDir = mkdtempSync(join(tmpdir(), 'gstack-state-test-'));
+});
+
+afterEach(() => {
+  rmSync(gstackDir, { recursive: true, force: true });
+  rmSync(stateDir, { recursive: true, force: true });
+});
+
+describe('gstack-update-check', () => {
+  // ─── Path A: No VERSION file ────────────────────────────────
+  test('exits 0 with no output when VERSION file is missing', () => {
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('');
+  });
+
+  // ─── Path B: Empty VERSION file ─────────────────────────────
+  test('exits 0 with no output when VERSION file is empty', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '');
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('');
+  });
+
+  // ─── Path C: Just-upgraded marker ───────────────────────────
+  test('outputs JUST_UPGRADED and deletes marker', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.4.0\n');
+    writeFileSync(join(stateDir, 'just-upgraded-from'), '0.3.3\n');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('JUST_UPGRADED 0.3.3 0.4.0');
+    // Marker should be deleted
+    expect(existsSync(join(stateDir, 'just-upgraded-from'))).toBe(false);
+    // Cache should be written
+    const cache = readFileSync(join(stateDir, 'last-update-check'), 'utf-8');
+    expect(cache).toContain('UP_TO_DATE');
+  });
+
+  // ─── Path D1: Fresh cache, UP_TO_DATE ───────────────────────
+  test('exits silently when cache says UP_TO_DATE and is fresh', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+    writeFileSync(join(stateDir, 'last-update-check'), 'UP_TO_DATE 0.3.3');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('');
+  });
+
+  // ─── Path D2: Fresh cache, UPGRADE_AVAILABLE ────────────────
+  test('echoes cached UPGRADE_AVAILABLE when cache is fresh', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+    writeFileSync(join(stateDir, 'last-update-check'), 'UPGRADE_AVAILABLE 0.3.3 0.4.0');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('UPGRADE_AVAILABLE 0.3.3 0.4.0');
+  });
+
+  // ─── Path D3: Fresh cache, but local version changed ────────
+  test('re-checks when local version does not match cached old version', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.4.0\n');
+    // Cache says 0.3.3 → 0.4.0 but we're already on 0.4.0
+    writeFileSync(join(stateDir, 'last-update-check'), 'UPGRADE_AVAILABLE 0.3.3 0.4.0');
+    // Remote also says 0.4.0 — should be up to date
+    writeFileSync(join(gstackDir, 'REMOTE_VERSION'), '0.4.0\n');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe(''); // Up to date after re-check
+    const cache = readFileSync(join(stateDir, 'last-update-check'), 'utf-8');
+    expect(cache).toContain('UP_TO_DATE');
+  });
+
+  // ─── Path E: Versions match (remote fetch) ─────────────────
+  test('writes UP_TO_DATE cache when versions match', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+    writeFileSync(join(gstackDir, 'REMOTE_VERSION'), '0.3.3\n');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('');
+    const cache = readFileSync(join(stateDir, 'last-update-check'), 'utf-8');
+    expect(cache).toContain('UP_TO_DATE');
+  });
+
+  // ─── Path F: Versions differ (remote fetch) ─────────────────
+  test('outputs UPGRADE_AVAILABLE when versions differ', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+    writeFileSync(join(gstackDir, 'REMOTE_VERSION'), '0.4.0\n');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('UPGRADE_AVAILABLE 0.3.3 0.4.0');
+    const cache = readFileSync(join(stateDir, 'last-update-check'), 'utf-8');
+    expect(cache).toContain('UPGRADE_AVAILABLE 0.3.3 0.4.0');
+  });
+
+  // ─── Path G: Invalid remote response ────────────────────────
+  test('treats invalid remote response as up to date', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+    writeFileSync(join(gstackDir, 'REMOTE_VERSION'), '<html>404 Not Found</html>\n');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('');
+    const cache = readFileSync(join(stateDir, 'last-update-check'), 'utf-8');
+    expect(cache).toContain('UP_TO_DATE');
+  });
+
+  // ─── Path H: Curl fails (bad URL) ──────────────────────────
+  test('exits silently when remote URL is unreachable', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+
+    const { exitCode, stdout } = run({
+      GSTACK_REMOTE_URL: 'file:///nonexistent/path/VERSION',
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('');
+    const cache = readFileSync(join(stateDir, 'last-update-check'), 'utf-8');
+    expect(cache).toContain('UP_TO_DATE');
+  });
+
+  // ─── Path I: Corrupt cache file ─────────────────────────────
+  test('falls through to remote fetch when cache is corrupt', () => {
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+    writeFileSync(join(stateDir, 'last-update-check'), 'garbage data here');
+    // Remote says same version — should end up UP_TO_DATE
+    writeFileSync(join(gstackDir, 'REMOTE_VERSION'), '0.3.3\n');
+
+    const { exitCode, stdout } = run();
+    expect(exitCode).toBe(0);
+    expect(stdout).toBe('');
+    // Cache should be overwritten with valid content
+    const cache = readFileSync(join(stateDir, 'last-update-check'), 'utf-8');
+    expect(cache).toContain('UP_TO_DATE');
+  });
+
+  // ─── State dir creation ─────────────────────────────────────
+  test('creates state dir if it does not exist', () => {
+    const newStateDir = join(stateDir, 'nested', 'dir');
+    writeFileSync(join(gstackDir, 'VERSION'), '0.3.3\n');
+    writeFileSync(join(gstackDir, 'REMOTE_VERSION'), '0.3.3\n');
+
+    const { exitCode } = run({ GSTACK_STATE_DIR: newStateDir });
+    expect(exitCode).toBe(0);
+    expect(existsSync(join(newStateDir, 'last-update-check'))).toBe(true);
+  });
+});