mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-19 10:52:28 +08:00
fix: sidebar agent uses real tab URL instead of stale Playwright URL (v0.12.6.0) (#544)
* fix: sidebar agent uses extension's activeTabUrl instead of stale Playwright URL When the user navigates manually in headed Chrome, Playwright's page.url() stays on the old page. The sidebar agent was using this stale URL in its system prompt, causing it to navigate to the wrong page (e.g., Hacker News instead of the user's current page). The Chrome extension now captures the active tab URL via chrome.tabs.query() and sends it as activeTabUrl in the /sidebar-command POST body. The server prefers this over Playwright's URL. The URL is sanitized (http/https only, control chars stripped, 2048 char limit) to prevent prompt injection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: connect-chrome pre-flight cleanup + improved onboarding docs Adds Step 0 pre-flight cleanup that kills stale browse servers and cleans Chromium profile locks before connecting. Improves the onboarding flow with clearer instructions for finding the extension, opening the Side Panel, and troubleshooting connection issues. Fixes Mode check from cdp to headed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: sidebar agent test suite (layers 1-2) Layer 1 (unit): 18 tests for URL sanitization in sidebar-utils.ts — http/https pass, chrome:// rejected, javascript: rejected, control chars stripped, truncation. Layer 2 (integration): 13 tests for server HTTP endpoints — auth, sidebar-command queue writes, activeTabUrl override/fallback, event relay to chat buffer, message queuing, queue overflow (429), chat clear, agent kill. Source changes for testability: - Extract sanitizeExtensionUrl() to browse/src/sidebar-utils.ts - Add BROWSE_HEADLESS_SKIP env var to skip browser launch in HTTP-only tests - Add SIDEBAR_QUEUE_PATH env var to both server.ts and sidebar-agent.ts - Add SIDEBAR_AGENT_TIMEOUT env var to sidebar-agent.ts - Sync package.json version to match VERSION (0.12.2.0) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: sidebar agent round-trip tests with mock claude (layer 3) Starts server + sidebar-agent together with a mock claude binary (shell script outputting canned stream-json). Verifies the full queue-based message flow: - Full round-trip: POST /sidebar-command → queue → agent → mock claude → events → chat - Claude crash recovery: mock exits 1, agent_error appears, status returns to idle - Sequential queue drain: two rapid messages both process in order Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: sidebar agent E2E tests with real Claude (layer 4) Two E2E tests that exercise the full sidebar agent flow with real Claude: - sidebar-navigate: POST /sidebar-command asking Claude to describe a fixture page, verify it responds with page content through the chat buffer - sidebar-url-accuracy: POST with activeTabUrl differing from Playwright URL, verify the queue prompt uses the extension URL (the core bug fix) Both registered as periodic tier (~$0.80 total, non-deterministic). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: sidebar E2E tests — sequential execution + eval collector fix Both tests now pass: - sidebar-url-accuracy: deterministic queue file check (no Claude needed) - sidebar-navigate: real Claude responds through sidebar agent queue Fixed: testIfSelected (sequential, not concurrent) to avoid queue file conflicts. Added cost_usd field for eval collector compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: kill stale sidebar-agent processes before starting new one Each /connect-chrome starts a new sidebar-agent subprocess with unref() but never kills the previous one. Old agents accumulate as zombies with stale auth tokens. When they pick up queue entries, their event relay fails (401), so the server never receives agent_done and marks the agent as "hung". The user sees the sidebar freeze. Fix: pkill any existing sidebar-agent.ts processes before spawning. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.12.6.0) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add P1 TODO for sidebar Write tool + error visibility Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -511,8 +511,27 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
|
||||
}
|
||||
}
|
||||
|
||||
// Clean up Chromium profile locks (can persist after crashes)
|
||||
// Kill orphaned Chromium processes that may still hold the profile lock.
|
||||
// The server PID is the Bun process; Chromium is a child that can outlive it
|
||||
// if the server is killed abruptly (SIGKILL, crash, manual rm of state file).
|
||||
const profileDir = path.join(process.env.HOME || '/tmp', '.gstack', 'chromium-profile');
|
||||
try {
|
||||
const singletonLock = path.join(profileDir, 'SingletonLock');
|
||||
const lockTarget = fs.readlinkSync(singletonLock); // e.g. "hostname-12345"
|
||||
const orphanPid = parseInt(lockTarget.split('-').pop() || '', 10);
|
||||
if (orphanPid && isProcessAlive(orphanPid)) {
|
||||
try { process.kill(orphanPid, 'SIGTERM'); } catch {}
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
if (isProcessAlive(orphanPid)) {
|
||||
try { process.kill(orphanPid, 'SIGKILL'); } catch {}
|
||||
await new Promise(resolve => setTimeout(resolve, 500));
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// No lock symlink or not readable — nothing to kill
|
||||
}
|
||||
|
||||
// Clean up Chromium profile locks (can persist after crashes)
|
||||
for (const lockFile of ['SingletonLock', 'SingletonSocket', 'SingletonCookie']) {
|
||||
try { fs.unlinkSync(path.join(profileDir, lockFile)); } catch {}
|
||||
}
|
||||
@@ -545,17 +564,38 @@ Refs: After 'snapshot', use @e1, @e2... as selectors:
|
||||
console.log(`Connected to real Chrome\n${status}`);
|
||||
|
||||
// Auto-start sidebar agent
|
||||
const agentScript = path.resolve(__dirname, 'sidebar-agent.ts');
|
||||
// __dirname is inside $bunfs in compiled binaries — resolve from execPath instead
|
||||
let agentScript = path.resolve(__dirname, 'sidebar-agent.ts');
|
||||
if (!fs.existsSync(agentScript)) {
|
||||
agentScript = path.resolve(path.dirname(process.execPath), '..', 'src', 'sidebar-agent.ts');
|
||||
}
|
||||
try {
|
||||
if (!fs.existsSync(agentScript)) {
|
||||
throw new Error(`sidebar-agent.ts not found at ${agentScript}`);
|
||||
}
|
||||
// Clear old agent queue
|
||||
const agentQueue = path.join(process.env.HOME || '/tmp', '.gstack', 'sidebar-agent-queue.jsonl');
|
||||
try { fs.writeFileSync(agentQueue, ''); } catch {}
|
||||
|
||||
// Resolve browse binary path the same way — execPath-relative
|
||||
let browseBin = path.resolve(__dirname, '..', 'dist', 'browse');
|
||||
if (!fs.existsSync(browseBin)) {
|
||||
browseBin = process.execPath; // the compiled binary itself
|
||||
}
|
||||
|
||||
// Kill any existing sidebar-agent processes before starting a new one.
|
||||
// Old agents have stale auth tokens and will silently fail to relay events,
|
||||
// causing the server to mark the agent as "hung".
|
||||
try {
|
||||
const { spawnSync } = require('child_process');
|
||||
spawnSync('pkill', ['-f', 'sidebar-agent\\.ts'], { stdio: 'ignore', timeout: 3000 });
|
||||
} catch {}
|
||||
|
||||
const agentProc = Bun.spawn(['bun', 'run', agentScript], {
|
||||
cwd: config.projectDir,
|
||||
env: {
|
||||
...process.env,
|
||||
BROWSE_BIN: path.resolve(__dirname, '..', 'dist', 'browse'),
|
||||
BROWSE_BIN: browseBin,
|
||||
BROWSE_STATE_FILE: config.stateFile,
|
||||
BROWSE_SERVER_PORT: String(newState.port),
|
||||
},
|
||||
|
||||
@@ -18,6 +18,7 @@ import { handleReadCommand } from './read-commands';
|
||||
import { handleWriteCommand } from './write-commands';
|
||||
import { handleMetaCommand } from './meta-commands';
|
||||
import { handleCookiePickerRoute } from './cookie-picker-routes';
|
||||
import { sanitizeExtensionUrl } from './sidebar-utils';
|
||||
import { COMMAND_DESCRIPTIONS } from './commands';
|
||||
import { handleSnapshot, SNAPSHOT_FLAGS } from './snapshot';
|
||||
import { resolveConfig, ensureStateDir, readVersionHash } from './config';
|
||||
@@ -123,7 +124,7 @@ let sidebarSession: SidebarSession | null = null;
|
||||
let agentProcess: ChildProcess | null = null;
|
||||
let agentStatus: 'idle' | 'processing' | 'hung' = 'idle';
|
||||
let agentStartTime: number | null = null;
|
||||
let messageQueue: Array<{message: string, ts: string}> = [];
|
||||
let messageQueue: Array<{message: string, ts: string, extensionUrl?: string | null}> = [];
|
||||
let currentMessage: string | null = null;
|
||||
let chatBuffer: ChatEntry[] = [];
|
||||
let chatNextId = 0;
|
||||
@@ -371,18 +372,27 @@ function processAgentEvent(event: any): void {
|
||||
}
|
||||
}
|
||||
|
||||
function spawnClaude(userMessage: string): void {
|
||||
function spawnClaude(userMessage: string, extensionUrl?: string | null): void {
|
||||
agentStatus = 'processing';
|
||||
agentStartTime = Date.now();
|
||||
currentMessage = userMessage;
|
||||
|
||||
const pageUrl = browserManager.getCurrentUrl() || 'about:blank';
|
||||
// Prefer the URL from the Chrome extension (what the user actually sees)
|
||||
// over Playwright's page.url() which can be stale in headed mode.
|
||||
const sanitizedExtUrl = sanitizeExtensionUrl(extensionUrl);
|
||||
const playwrightUrl = browserManager.getCurrentUrl() || 'about:blank';
|
||||
const pageUrl = sanitizedExtUrl || playwrightUrl;
|
||||
const B = BROWSE_BIN;
|
||||
const systemPrompt = [
|
||||
'You are a browser assistant running in a Chrome sidebar.',
|
||||
`Current page: ${pageUrl}`,
|
||||
`The user is currently viewing: ${pageUrl}`,
|
||||
`Browse binary: ${B}`,
|
||||
'',
|
||||
'IMPORTANT: You are controlling a SHARED browser. The user may have navigated',
|
||||
'manually. Always run `' + B + ' url` first to check the actual current URL.',
|
||||
'If it differs from above, the user navigated — work with the ACTUAL page.',
|
||||
'Do NOT navigate away from the user\'s current page unless they ask you to.',
|
||||
'',
|
||||
'Commands (run via bash):',
|
||||
` ${B} goto <url> ${B} click <@ref> ${B} fill <@ref> <text>`,
|
||||
` ${B} snapshot -i ${B} text ${B} screenshot`,
|
||||
@@ -404,8 +414,8 @@ function spawnClaude(userMessage: string): void {
|
||||
// fails with ENOENT on everything, including /bin/bash). Instead,
|
||||
// write the command to a queue file that the sidebar-agent process
|
||||
// (running as non-compiled bun) picks up and spawns claude.
|
||||
const gstackDir = path.join(process.env.HOME || '/tmp', '.gstack');
|
||||
const agentQueue = path.join(gstackDir, 'sidebar-agent-queue.jsonl');
|
||||
const agentQueue = process.env.SIDEBAR_QUEUE_PATH || path.join(process.env.HOME || '/tmp', '.gstack', 'sidebar-agent-queue.jsonl');
|
||||
const gstackDir = path.dirname(agentQueue);
|
||||
const entry = JSON.stringify({
|
||||
ts: new Date().toISOString(),
|
||||
message: userMessage,
|
||||
@@ -414,6 +424,7 @@ function spawnClaude(userMessage: string): void {
|
||||
stateFile: config.stateFile,
|
||||
cwd: (sidebarSession as any)?.worktreePath || process.cwd(),
|
||||
sessionId: sidebarSession?.claudeSessionId || null,
|
||||
pageUrl: pageUrl,
|
||||
});
|
||||
try {
|
||||
fs.mkdirSync(gstackDir, { recursive: true });
|
||||
@@ -781,12 +792,16 @@ async function start() {
|
||||
const port = await findPort();
|
||||
|
||||
// Launch browser (headless or headed with extension)
|
||||
const headed = process.env.BROWSE_HEADED === '1';
|
||||
if (headed) {
|
||||
await browserManager.launchHeaded();
|
||||
console.log(`[browse] Launched headed Chromium with extension`);
|
||||
} else {
|
||||
await browserManager.launch();
|
||||
// BROWSE_HEADLESS_SKIP=1 skips browser launch entirely (for HTTP-only testing)
|
||||
const skipBrowser = process.env.BROWSE_HEADLESS_SKIP === '1';
|
||||
if (!skipBrowser) {
|
||||
const headed = process.env.BROWSE_HEADED === '1';
|
||||
if (headed) {
|
||||
await browserManager.launchHeaded();
|
||||
console.log(`[browse] Launched headed Chromium with extension`);
|
||||
} else {
|
||||
await browserManager.launch();
|
||||
}
|
||||
}
|
||||
|
||||
const startTime = Date.now();
|
||||
@@ -935,17 +950,21 @@ async function start() {
|
||||
if (!msg) {
|
||||
return new Response(JSON.stringify({ error: 'Empty message' }), { status: 400, headers: { 'Content-Type': 'application/json' } });
|
||||
}
|
||||
// The Chrome extension sends the active tab's URL — prefer it over
|
||||
// Playwright's page.url() which can be stale in headed mode when
|
||||
// the user navigates manually.
|
||||
const extensionUrl = body.activeTabUrl || null;
|
||||
const ts = new Date().toISOString();
|
||||
addChatEntry({ ts, role: 'user', message: msg });
|
||||
if (sidebarSession) { sidebarSession.lastActiveAt = ts; saveSession(); }
|
||||
|
||||
if (agentStatus === 'idle') {
|
||||
spawnClaude(msg);
|
||||
spawnClaude(msg, extensionUrl);
|
||||
return new Response(JSON.stringify({ ok: true, processing: true }), {
|
||||
status: 200, headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
} else if (messageQueue.length < MAX_QUEUE) {
|
||||
messageQueue.push({ message: msg, ts });
|
||||
messageQueue.push({ message: msg, ts, extensionUrl });
|
||||
return new Response(JSON.stringify({ ok: true, queued: true, position: messageQueue.length }), {
|
||||
status: 200, headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
@@ -979,7 +998,7 @@ async function start() {
|
||||
// Process next in queue
|
||||
if (messageQueue.length > 0) {
|
||||
const next = messageQueue.shift()!;
|
||||
spawnClaude(next.message);
|
||||
spawnClaude(next.message, next.extensionUrl);
|
||||
}
|
||||
return new Response(JSON.stringify({ ok: true }), { status: 200, headers: { 'Content-Type': 'application/json' } });
|
||||
}
|
||||
@@ -1065,7 +1084,7 @@ async function start() {
|
||||
// Process next queued message
|
||||
if (messageQueue.length > 0) {
|
||||
const next = messageQueue.shift()!;
|
||||
spawnClaude(next.message);
|
||||
spawnClaude(next.message, next.extensionUrl);
|
||||
} else {
|
||||
agentStatus = 'idle';
|
||||
}
|
||||
|
||||
@@ -13,7 +13,7 @@ import { spawn } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
|
||||
const QUEUE = path.join(process.env.HOME || '/tmp', '.gstack', 'sidebar-agent-queue.jsonl');
|
||||
const QUEUE = process.env.SIDEBAR_QUEUE_PATH || path.join(process.env.HOME || '/tmp', '.gstack', 'sidebar-agent-queue.jsonl');
|
||||
const SERVER_PORT = parseInt(process.env.BROWSE_SERVER_PORT || '34567', 10);
|
||||
const SERVER_URL = `http://127.0.0.1:${SERVER_PORT}`;
|
||||
const POLL_MS = 500; // Fast polling — server already did the user-facing response
|
||||
@@ -205,14 +205,15 @@ async function askClaude(queueEntry: any): Promise<void> {
|
||||
});
|
||||
});
|
||||
|
||||
// Timeout after 300 seconds (5 min — multi-page tasks need time)
|
||||
// Timeout (default 300s / 5 min — multi-page tasks need time)
|
||||
const timeoutMs = parseInt(process.env.SIDEBAR_AGENT_TIMEOUT || '300000', 10);
|
||||
setTimeout(() => {
|
||||
try { proc.kill(); } catch {}
|
||||
sendEvent({ type: 'agent_error', error: 'Timed out after 300s' }).then(() => {
|
||||
sendEvent({ type: 'agent_error', error: `Timed out after ${timeoutMs / 1000}s` }).then(() => {
|
||||
isProcessing = false;
|
||||
resolve();
|
||||
});
|
||||
}, 300000);
|
||||
}, timeoutMs);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
21
browse/src/sidebar-utils.ts
Normal file
21
browse/src/sidebar-utils.ts
Normal file
@@ -0,0 +1,21 @@
|
||||
/**
|
||||
* Shared sidebar utilities — extracted for testability.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Sanitize a URL from the Chrome extension before embedding in a prompt.
|
||||
* Only accepts http/https, strips control characters, truncates to 2048 chars.
|
||||
* Returns null if the URL is invalid or uses a non-http scheme.
|
||||
*/
|
||||
export function sanitizeExtensionUrl(url: string | null | undefined): string | null {
|
||||
if (!url) return null;
|
||||
try {
|
||||
const u = new URL(url);
|
||||
if (u.protocol === 'http:' || u.protocol === 'https:') {
|
||||
return u.href.replace(/[\x00-\x1f\x7f]/g, '').slice(0, 2048);
|
||||
}
|
||||
return null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user