diff --git a/.github/workflows/e2e-headed.yml b/.github/workflows/e2e-headed.yml index 6e0c2d9..1fbecca 100644 --- a/.github/workflows/e2e-headed.yml +++ b/.github/workflows/e2e-headed.yml @@ -31,7 +31,9 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + # Pin macOS instead of following macos-latest while hosted macOS 26 + # browser/runtime behavior remains less predictable. + os: [ubuntu-latest, macos-15] steps: - uses: actions/checkout@v6 diff --git a/clis/twitter/delete.js b/clis/twitter/delete.js index 1258e80..3736558 100644 --- a/clis/twitter/delete.js +++ b/clis/twitter/delete.js @@ -1,28 +1,36 @@ import { cli, Strategy } from '@agentrhq/webcmd/registry'; import { CommandExecutionError } from '@agentrhq/webcmd/errors'; -import { parseTweetUrl, buildTwitterArticleScopeSource } from './shared.js'; +import { parseTweetUrl, buildTwitterArticleScopeSource, unwrapBrowserResult } from './shared.js'; function buildDeleteScript(tweetId) { return `(async () => { try { const visible = (el) => !!el && (el.offsetParent !== null || el.getClientRects().length > 0); ${buildTwitterArticleScopeSource(tweetId)} - const targetArticle = findTargetArticle(); + let targetArticle = findTargetArticle(); + for (let i = 0; i < 20 && !targetArticle; i++) { + await new Promise(r => setTimeout(r, 250)); + targetArticle = findTargetArticle(); + } if (!targetArticle) { return { ok: false, message: 'Could not find the tweet card matching the requested URL.' }; } - const buttons = Array.from(targetArticle.querySelectorAll('button,[role="button"]')); - const moreMenu = buttons.find((el) => visible(el) && (el.getAttribute('aria-label') || '').trim() === 'More'); + const belongsToTargetArticle = (el) => el.closest('article') === targetArticle; + const buttons = Array.from(targetArticle.querySelectorAll('button,[role="button"]')).filter(belongsToTargetArticle); + const moreMenu = Array.from(targetArticle.querySelectorAll('[data-testid="caret"]')).filter(belongsToTargetArticle).find(visible) + || buttons.find((el) => visible(el) && /^More/.test((el.getAttribute('aria-label') || '').trim())); if (!moreMenu) { return { ok: false, message: 'Could not find the "More" context menu on the matched tweet. Are you sure you are logged in and looking at a valid tweet?' }; } + const beforeMenuItems = new Set(document.querySelectorAll('[role="menuitem"]')); moreMenu.click(); await new Promise(r => setTimeout(r, 1000)); - const items = Array.from(document.querySelectorAll('[role="menuitem"]')); + const items = Array.from(document.querySelectorAll('[role="menuitem"]')) + .filter((item) => visible(item) && !beforeMenuItems.has(item)); const deleteBtn = items.find((item) => { const text = (item.textContent || '').trim(); return text.includes('Delete') && !text.includes('List'); @@ -69,7 +77,7 @@ cli({ const target = parseTweetUrl(kwargs.url); await page.goto(target.url); await page.wait({ selector: '[data-testid="primaryColumn"]' }); // Wait for tweet to load completely - const result = await page.evaluate(buildDeleteScript(target.id)); + const result = unwrapBrowserResult(await page.evaluate(buildDeleteScript(target.id))); if (result.ok) { // Wait for the deletion request to be processed await page.wait(2); diff --git a/clis/twitter/delete.test.js b/clis/twitter/delete.test.js index f52c9e8..74a0853 100644 --- a/clis/twitter/delete.test.js +++ b/clis/twitter/delete.test.js @@ -1,7 +1,8 @@ import { describe, expect, it, vi } from 'vitest'; +import { JSDOM } from 'jsdom'; import { ArgumentError, CommandExecutionError } from '@agentrhq/webcmd/errors'; import { getRegistry } from '@agentrhq/webcmd/registry'; -import './delete.js'; +import { __test__ } from './delete.js'; describe('twitter delete command', () => { it('targets the matched tweet article instead of the first More button on the page', async () => { const cmd = getRegistry().get('twitter/delete'); @@ -27,6 +28,12 @@ describe('twitter delete command', () => { expect(script).toContain('__twGetStatusIdFromHref'); expect(script).toContain("document.querySelectorAll('article')"); expect(script).toContain("targetArticle.querySelectorAll('button,[role=\"button\"]')"); + expect(script).toContain("closest('article') === targetArticle"); + expect(script).toContain(".filter(belongsToTargetArticle)"); + expect(script).toContain('[data-testid="caret"]'); + expect(script).toContain('/^More/'); + expect(script).toContain('i < 20'); + expect(script).toContain('beforeMenuItems'); // Substring match must NOT appear — exact-id match only. expect(script).not.toContain("'/status/' + tweetId"); expect(result).toEqual([ @@ -58,6 +65,72 @@ describe('twitter delete command', () => { ]); expect(page.wait).toHaveBeenCalledTimes(1); }); + it('unwraps browser runtime evaluate envelopes before checking delete success', async () => { + const cmd = getRegistry().get('twitter/delete'); + expect(cmd?.func).toBeTypeOf('function'); + const page = { + goto: vi.fn().mockResolvedValue(undefined), + wait: vi.fn().mockResolvedValue(undefined), + evaluate: vi.fn().mockResolvedValue({ + session: 'twitter', + data: { ok: true, message: 'Tweet successfully deleted.' }, + }), + }; + const result = await cmd.func(page, { + url: 'https://x.com/alice/status/2040254679301718161', + }); + expect(result).toEqual([ + { + status: 'success', + message: 'Tweet successfully deleted.', + }, + ]); + expect(page.wait).toHaveBeenNthCalledWith(2, 2); + }); + it('ignores stale non-target delete menu items that existed before opening the matched tweet menu', async () => { + const dom = new JSDOM(` + +
Delete
+
+ wrong tweet + +
+
+ target tweet + +
+ + `, { runScripts: 'outside-only', url: 'https://x.com/alice/status/2040254679301718161' }); + dom.window.setTimeout = (handler) => { + if (typeof handler === 'function') handler(); + return 0; + }; + Object.defineProperty(dom.window.HTMLElement.prototype, 'getClientRects', { + configurable: true, + value() { + return [{ bottom: 1, height: 1, left: 0, right: 1, top: 0, width: 1 }]; + }, + }); + let staleDeleteClicked = false; + let targetCaretClicked = false; + dom.window.document.querySelector('[data-stale-delete]')?.addEventListener('click', () => { + staleDeleteClicked = true; + }); + dom.window.document.querySelector('[data-target-caret]')?.addEventListener('click', () => { + targetCaretClicked = true; + const item = dom.window.document.createElement('div'); + item.setAttribute('role', 'menuitem'); + item.textContent = 'Pin to your profile'; + dom.window.document.body.appendChild(item); + }); + const result = await dom.window.eval(__test__.buildDeleteScript('2040254679301718161')); + expect(targetCaretClicked).toBe(true); + expect(staleDeleteClicked).toBe(false); + expect(result).toEqual({ + ok: false, + message: 'The matched tweet menu did not contain Delete. This tweet may not belong to you.', + }); + }); it('rejects malformed or off-domain URLs with ArgumentError before navigation', async () => { const cmd = getRegistry().get('twitter/delete'); expect(cmd?.func).toBeTypeOf('function'); diff --git a/src/browser/bridge.ts b/src/browser/bridge.ts index c5654a0..96320c3 100644 --- a/src/browser/bridge.ts +++ b/src/browser/bridge.ts @@ -6,7 +6,7 @@ import type { ChildProcess } from 'node:child_process'; import type { IPage } from '../types.js'; import type { IBrowserFactory } from '../runtime.js'; import { Page } from './page.js'; -import { resolveProfileContextId } from './profile.js'; +import { profileRouteParams, resolveProfileSelection } from './profile.js'; import { ensureBrowserBridgeReady } from './daemon-lifecycle.js'; const DAEMON_SPAWN_TIMEOUT = 10000; // 10s to wait for daemon + extension @@ -25,7 +25,7 @@ export class BrowserBridge implements IBrowserFactory { return this._state; } - async connect(opts: { timeout?: number; session?: string; idleTimeout?: number; contextId?: string; windowMode?: 'foreground' | 'background'; surface?: 'browser' | 'adapter'; siteSession?: 'ephemeral' | 'persistent' } = {}): Promise { + async connect(opts: { timeout?: number; session?: string; idleTimeout?: number; contextId?: string; preferredContextId?: string; windowMode?: 'foreground' | 'background'; surface?: 'browser' | 'adapter'; siteSession?: 'ephemeral' | 'persistent' } = {}): Promise { if (this._state === 'connected' && this._page) return this._page; if (this._state === 'connecting') throw new Error('Already connecting'); if (this._state === 'closing') throw new Error('Session is closing'); @@ -34,10 +34,12 @@ export class BrowserBridge implements IBrowserFactory { this._state = 'connecting'; try { - const contextId = opts.contextId ?? resolveProfileContextId(); - await this._ensureDaemon(opts.timeout, contextId); + const routing = opts.contextId || opts.preferredContextId + ? { contextId: opts.contextId, preferredContextId: opts.preferredContextId } + : profileRouteParams(resolveProfileSelection()); + await this._ensureDaemon(opts.timeout, routing.contextId); if (!opts.session?.trim()) throw new Error('Browser session is required'); - this._page = new Page(opts.session.trim(), opts.idleTimeout, contextId, opts.windowMode, opts.surface, opts.siteSession); + this._page = new Page(opts.session.trim(), opts.idleTimeout, routing.contextId, opts.windowMode, opts.surface, opts.siteSession, routing.preferredContextId); this._state = 'connected'; return this._page; } catch (err) { diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index 12ba2cb..1006669 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -53,7 +53,7 @@ export class CDPBridge implements IBrowserFactory { private _pending = new Map void; reject: (err: Error) => void; timer: ReturnType }>(); private _eventListeners = new Map void>>(); - async connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; idleTimeout?: number; windowMode?: 'foreground' | 'background'; surface?: 'browser' | 'adapter'; siteSession?: 'ephemeral' | 'persistent' }): Promise { + async connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; preferredContextId?: string; idleTimeout?: number; windowMode?: 'foreground' | 'background'; surface?: 'browser' | 'adapter'; siteSession?: 'ephemeral' | 'persistent' }): Promise { if (this._ws) throw new Error('CDPBridge is already connected. Call close() before reconnecting.'); const endpoint = opts?.cdpEndpoint ?? process.env.WEBCMD_CDP_ENDPOINT; diff --git a/src/browser/daemon-client.test.ts b/src/browser/daemon-client.test.ts index 0ff0994..7fb5836 100644 --- a/src/browser/daemon-client.test.ts +++ b/src/browser/daemon-client.test.ts @@ -159,29 +159,6 @@ describe('daemon-client', () => { expect(vi.mocked(fetch).mock.calls[0][0]).toMatch(/\/status\?contextId=work$/); }); - it('rejects WEBCMD_DAEMON_PORT so CLI and extension cannot split bridge ports', async () => { - vi.resetModules(); - vi.stubEnv('WEBCMD_DAEMON_PORT', '19999'); - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: () => Promise.resolve({ - ok: true, - pid: 1, - uptime: 0, - runtimeConnected: true, - runtimeName: 'fake', - pending: 0, - memoryMB: 1, - port: 9777, - }), - } as Response); - - const freshClient = await import('./daemon-client.js'); - await expect(freshClient.fetchDaemonStatus()).rejects.toThrow('WEBCMD_DAEMON_PORT is no longer supported'); - - expect(vi.mocked(fetch)).not.toHaveBeenCalled(); - }); - it('sendCommand includes the current pid in generated command ids', async () => { vi.spyOn(Date, 'now').mockReturnValue(1_763_000_000_000); vi.mocked(fetch).mockResolvedValue({ @@ -203,7 +180,7 @@ describe('daemon-client', () => { expect(ids[0]).not.toBe(ids[1]); }); - it('sendCommand forwards WEBCMD_PROFILE as command contextId', async () => { + it('sendCommand forwards WEBCMD_PROFILE as a hard contextId requirement', async () => { vi.stubEnv('WEBCMD_PROFILE', 'work'); vi.spyOn(Date, 'now').mockReturnValue(1_763_000_000_000); vi.mocked(fetch).mockResolvedValue({ @@ -213,8 +190,36 @@ describe('daemon-client', () => { await sendCommand('exec', { code: '1 + 1' }); - const body = JSON.parse(String(vi.mocked(fetch).mock.calls[0][1]?.body)) as { contextId?: string }; + const body = JSON.parse(String(vi.mocked(fetch).mock.calls[0][1]?.body)) as { contextId?: string; preferredContextId?: string }; expect(body.contextId).toBe('work'); + expect(body.preferredContextId).toBeUndefined(); + }); + + it('sendCommand forwards the config default as preferredContextId', async () => { + const fs = await import('node:fs'); + const os = await import('node:os'); + const path = await import('node:path'); + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'webcmd-dc-profile-')); + fs.writeFileSync( + path.join(configDir, 'browser-profiles.json'), + JSON.stringify({ version: 1, aliases: {}, defaultContextId: 'profile-default' }), + ); + vi.stubEnv('WEBCMD_CONFIG_DIR', configDir); + vi.stubEnv('WEBCMD_PROFILE', ''); + try { + vi.mocked(fetch).mockResolvedValue({ + status: 200, + json: () => Promise.resolve({ id: 'server', ok: true, data: 'ok' }), + } as Response); + + await sendCommand('exec', { code: '1 + 1' }); + + const body = JSON.parse(String(vi.mocked(fetch).mock.calls[0][1]?.body)) as { contextId?: string; preferredContextId?: string }; + expect(body.contextId).toBeUndefined(); + expect(body.preferredContextId).toBe('profile-default'); + } finally { + fs.rmSync(configDir, { recursive: true, force: true }); + } }); it('sendCommand uses explicit windowMode before WEBCMD_WINDOW env fallback', async () => { diff --git a/src/browser/daemon-client.ts b/src/browser/daemon-client.ts index dfdf289..3aafab5 100644 --- a/src/browser/daemon-client.ts +++ b/src/browser/daemon-client.ts @@ -8,7 +8,7 @@ import { sleep } from '../utils.js'; import { BrowserConnectError } from '../errors.js'; import { COMMAND_RESULT_UNKNOWN_CODE, COMMAND_RESULT_UNKNOWN_HINT } from '../daemon-utils.js'; import { classifyBrowserError } from './errors.js'; -import { resolveProfileContextId } from './profile.js'; +import { profileRouteParams, resolveProfileSelection } from './profile.js'; import { DEFAULT_BROWSER_CONNECT_TIMEOUT } from './config.js'; import { ensureBrowserBridgeReady } from './daemon-lifecycle.js'; import { isPreDispatchError } from './bridge-readiness.js'; @@ -117,7 +117,11 @@ async function sendCommandRaw( const envWindowMode = rawWindowMode === 'foreground' || rawWindowMode === 'background' ? rawWindowMode : undefined; - const contextId = params.contextId ?? resolveProfileContextId(); + const routing = params.contextId || params.preferredContextId + ? { contextId: params.contextId, preferredContextId: params.preferredContextId } + : profileRouteParams(resolveProfileSelection()); + const contextId = routing.contextId; + const preferredContextId = routing.preferredContextId; const windowMode = params.windowMode ?? envWindowMode; let id = generateId(); @@ -150,6 +154,7 @@ async function sendCommandRaw( timeout: timeoutSeconds, deadlineAt, ...(contextId && { contextId }), + ...(preferredContextId && { preferredContextId }), ...(windowMode && { windowMode }), }; try { @@ -245,6 +250,6 @@ export async function sendCommandFull( return { data: result.data, page: result.page }; } -export async function bindTab(session: string, opts: { contextId?: string; page?: string; index?: number } = {}): Promise { +export async function bindTab(session: string, opts: { contextId?: string; preferredContextId?: string; page?: string; index?: number } = {}): Promise { return sendCommand('bind', { session, surface: 'browser', ...opts }); } diff --git a/src/browser/daemon-transport.ts b/src/browser/daemon-transport.ts index 3ac67e0..a147db7 100644 --- a/src/browser/daemon-transport.ts +++ b/src/browser/daemon-transport.ts @@ -1,21 +1,9 @@ -import { DAEMON_HEADER_NAME, DEFAULT_DAEMON_PORT, unsupportedDaemonPortEnvMessage } from '../constants.js'; +import { DAEMON_HEADER_NAME, DEFAULT_DAEMON_PORT } from '../constants.js'; const DAEMON_PORT = DEFAULT_DAEMON_PORT; const DAEMON_URL = `http://127.0.0.1:${DAEMON_PORT}`; const WEBCMD_HEADERS = { [DAEMON_HEADER_NAME]: '1' }; -class UnsupportedDaemonPortEnvError extends Error { - constructor(value: string) { - super(unsupportedDaemonPortEnvMessage(value)); - this.name = 'UnsupportedDaemonPortEnvError'; - } -} - -function assertSupportedDaemonPortEnv(): void { - const value = process.env.WEBCMD_DAEMON_PORT; - if (value !== undefined && value !== '') throw new UnsupportedDaemonPortEnvError(value); -} - export interface DaemonStatus { ok: boolean; pid: number; @@ -50,7 +38,6 @@ export type DaemonHealth = | { state: 'ready'; status: DaemonStatus }; export async function requestDaemon(pathname: string, init?: RequestInit & { timeout?: number }): Promise { - assertSupportedDaemonPortEnv(); const { timeout = 2000, headers, ...rest } = init ?? {}; const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), timeout); @@ -71,8 +58,7 @@ export async function fetchDaemonStatus(opts?: { timeout?: number; contextId?: s const res = await requestDaemon(`/status${params}`, { timeout: opts?.timeout ?? 2000 }); if (!res.ok) return null; return await res.json() as DaemonStatus; - } catch (err) { - if (err instanceof UnsupportedDaemonPortEnvError) throw err; + } catch { return null; } } @@ -90,8 +76,7 @@ export async function requestDaemonShutdown(opts?: { timeout?: number }): Promis try { const res = await requestDaemon('/shutdown', { method: 'POST', timeout: opts?.timeout ?? 5000 }); return res.ok; - } catch (err) { - if (err instanceof UnsupportedDaemonPortEnvError) throw err; + } catch { return false; } } diff --git a/src/browser/page.ts b/src/browser/page.ts index d68ea96..fed41c4 100644 --- a/src/browser/page.ts +++ b/src/browser/page.ts @@ -49,6 +49,7 @@ export class Page extends BasePage { private readonly windowMode?: 'foreground' | 'background', private readonly surface: 'browser' | 'adapter' = 'browser', private readonly siteSession?: 'ephemeral' | 'persistent', + public readonly preferredContextId?: string, ) { super(); this._idleTimeout = idleTimeout; @@ -60,11 +61,12 @@ export class Page extends BasePage { private _networkCaptureWarned = false; /** Helper: spread session into command params */ - private _sessionOpts(): { session: string; surface: 'browser' | 'adapter'; idleTimeout?: number; contextId?: string; windowMode?: 'foreground' | 'background'; siteSession?: 'ephemeral' | 'persistent' } { + private _sessionOpts(): { session: string; surface: 'browser' | 'adapter'; idleTimeout?: number; contextId?: string; preferredContextId?: string; windowMode?: 'foreground' | 'background'; siteSession?: 'ephemeral' | 'persistent' } { return { session: this.session, surface: this.surface, ...(this.contextId && { contextId: this.contextId }), + ...(this.preferredContextId && { preferredContextId: this.preferredContextId }), ...(this._idleTimeout != null && { idleTimeout: this._idleTimeout }), ...(this.windowMode && { windowMode: this.windowMode }), ...(this.siteSession && { siteSession: this.siteSession }), @@ -77,6 +79,7 @@ export class Page extends BasePage { session: this.session, surface: this.surface, ...(this.contextId && { contextId: this.contextId }), + ...(this.preferredContextId && { preferredContextId: this.preferredContextId }), ...(this._page !== undefined && { page: this._page }), ...(this._idleTimeout != null && { idleTimeout: this._idleTimeout }), ...(this.windowMode && { windowMode: this.windowMode }), diff --git a/src/browser/profile.test.ts b/src/browser/profile.test.ts new file mode 100644 index 0000000..93e4e08 --- /dev/null +++ b/src/browser/profile.test.ts @@ -0,0 +1,53 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { ENV_PREFIX } from '../brand.js'; +import { profileRouteParams, resolveProfileSelection } from './profile.js'; + +describe('profile selection', () => { + let configDir: string; + + beforeEach(() => { + configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'webcmd-profile-test-')); + vi.stubEnv(`${ENV_PREFIX}_CONFIG_DIR`, configDir); + vi.stubEnv(`${ENV_PREFIX}_PROFILE`, ''); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + fs.rmSync(configDir, { recursive: true, force: true }); + }); + + function writeConfig(config: object): void { + fs.writeFileSync(path.join(configDir, 'browser-profiles.json'), JSON.stringify(config)); + } + + it('tags an explicit profile argument as explicit and resolves aliases', () => { + writeConfig({ version: 1, aliases: { work: 'profile-work' } }); + expect(resolveProfileSelection('work')).toEqual({ contextId: 'profile-work', source: 'explicit' }); + }); + + it('tags WEBCMD_PROFILE as explicit', () => { + vi.stubEnv(`${ENV_PREFIX}_PROFILE`, 'profile-env'); + expect(resolveProfileSelection()).toEqual({ contextId: 'profile-env', source: 'explicit' }); + }); + + it('tags the persisted config default as preferred', () => { + writeConfig({ version: 1, aliases: {}, defaultContextId: 'profile-default' }); + expect(resolveProfileSelection()).toEqual({ contextId: 'profile-default', source: 'preferred' }); + }); + + it('explicit argument beats env, and env beats config default', () => { + vi.stubEnv(`${ENV_PREFIX}_PROFILE`, 'from-env'); + writeConfig({ version: 1, aliases: {}, defaultContextId: 'from-config' }); + expect(resolveProfileSelection('from-arg')).toEqual({ contextId: 'from-arg', source: 'explicit' }); + expect(resolveProfileSelection()).toEqual({ contextId: 'from-env', source: 'explicit' }); + }); + + it('maps explicit routes to contextId and preferred routes to preferredContextId', () => { + expect(profileRouteParams({ contextId: 'a', source: 'explicit' })).toEqual({ contextId: 'a' }); + expect(profileRouteParams({ contextId: 'b', source: 'preferred' })).toEqual({ preferredContextId: 'b' }); + expect(profileRouteParams(undefined)).toEqual({}); + }); +}); diff --git a/src/browser/profile.ts b/src/browser/profile.ts index 89abac4..37e8ba4 100644 --- a/src/browser/profile.ts +++ b/src/browser/profile.ts @@ -54,13 +54,31 @@ export function saveProfileConfig(config: ProfileConfig): void { fs.writeFileSync(target, JSON.stringify(config, null, 2) + '\n', 'utf-8'); } -export function resolveProfileContextId(profile?: string): string | undefined { +export type ProfileSelection = { + contextId: string; + source: 'explicit' | 'preferred'; +}; + +export function resolveProfileSelection(profile?: string): ProfileSelection | undefined { const config = loadProfileConfig(); - const requested = normalizeContextId(profile) - ?? normalizeContextId(process.env[`${ENV_PREFIX}_PROFILE`]) - ?? normalizeContextId(config.defaultContextId); - if (!requested) return undefined; - return config.aliases[requested] ?? requested; + const explicit = normalizeContextId(profile) ?? normalizeContextId(process.env[`${ENV_PREFIX}_PROFILE`]); + if (explicit) return { contextId: config.aliases[explicit] ?? explicit, source: 'explicit' }; + const preferred = normalizeContextId(config.defaultContextId); + if (preferred) return { contextId: config.aliases[preferred] ?? preferred, source: 'preferred' }; + return undefined; +} + +export function profileRouteParams( + selection: ProfileSelection | undefined, +): { contextId?: string; preferredContextId?: string } { + if (!selection) return {}; + return selection.source === 'explicit' + ? { contextId: selection.contextId } + : { preferredContextId: selection.contextId }; +} + +export function resolveProfileContextId(profile?: string): string | undefined { + return resolveProfileSelection(profile)?.contextId; } export function aliasForContextId(config: ProfileConfig, contextId: string): string | undefined { diff --git a/src/browser/protocol.ts b/src/browser/protocol.ts index 0fe6170..6b2687f 100644 --- a/src/browser/protocol.ts +++ b/src/browser/protocol.ts @@ -50,6 +50,7 @@ export interface BrowserRuntimeCommand { idleTimeout?: number; frameIndex?: number; contextId?: string; + preferredContextId?: string; profileId?: string; } diff --git a/src/browser/runtime/local-cloak/actions.ts b/src/browser/runtime/local-cloak/actions.ts index 8200daa..cbfbe22 100644 --- a/src/browser/runtime/local-cloak/actions.ts +++ b/src/browser/runtime/local-cloak/actions.ts @@ -14,8 +14,25 @@ class CloakActionError extends Error { } } -function commandProfileId(command: BrowserRuntimeCommand): string { - return command.profileId ?? command.contextId ?? 'default'; +function commandProfileId(manager: CloakSessionManager, command: BrowserRuntimeCommand): string { + const requested = command.profileId ?? command.contextId; + if (requested?.trim()) return requested.trim(); + + const preferred = command.preferredContextId?.trim(); + if (!preferred) return 'default'; + + const active = manager.activeProfileIds(); + if (active.includes(preferred)) return preferred; + if (active.length === 1) return active[0]; + if (active.length > 1) { + throw new CloakActionError( + 'profile_required', + `Default Cloak profile "${preferred}" is not active and multiple profiles are running; choose one with --profile.`, + undefined, + 'Run webcmd profile list, then update the default with webcmd profile use or pass --profile .', + ); + } + return preferred; } function invalidRequest(command: BrowserRuntimeCommand, error: string): BrowserRuntimeResult { @@ -29,7 +46,7 @@ async function resolveLease(manager: CloakSessionManager, command: BrowserRuntim throw new CloakActionError('stale_page_identity', `Page not found: ${command.page} — stale page identity`); } return manager.getPage({ - profileId: commandProfileId(command), + profileId: commandProfileId(manager, command), session: command.session, surface: command.surface, siteSession: command.siteSession, @@ -94,11 +111,11 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command: } case 'close-window': { if (command.page) { - const closed = await manager.closePage({ profileId: commandProfileId(command), pageId: command.page }); + const closed = await manager.closePage({ profileId: commandProfileId(manager, command), pageId: command.page }); return { id: command.id, ok: true, data: { closed: Boolean(closed), page: closed ?? command.page, session: command.session } }; } else { await manager.release({ - profileId: commandProfileId(command), + profileId: commandProfileId(manager, command), session: command.session, surface: command.surface, }); @@ -108,12 +125,12 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command: case 'tabs': { switch (command.op ?? 'list') { case 'list': { - const tabs = await manager.listPages({ profileId: commandProfileId(command) }); + const tabs = await manager.listPages({ profileId: commandProfileId(manager, command) }); return { id: command.id, ok: true, data: tabs }; } case 'new': { const lease = await manager.newPage({ - profileId: commandProfileId(command), + profileId: commandProfileId(manager, command), session: command.session, surface: command.surface, siteSession: command.siteSession, @@ -123,12 +140,12 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command: return { id: command.id, ok: true, data: { title: await lease.page.title(), url: lease.page.url() }, page: lease.pageId }; } case 'select': { - const lease = await manager.selectPage({ profileId: commandProfileId(command), pageId: command.page, index: command.index }); + const lease = await manager.selectPage({ profileId: commandProfileId(manager, command), pageId: command.page, index: command.index }); if (!lease) return { id: command.id, ok: false, errorCode: 'runtime_command_failed', error: 'Tab not found' }; return { id: command.id, ok: true, data: { selected: true, url: lease.page.url() }, page: lease.pageId }; } case 'close': { - const closed = await manager.closePage({ profileId: commandProfileId(command), pageId: command.page, index: command.index }); + const closed = await manager.closePage({ profileId: commandProfileId(manager, command), pageId: command.page, index: command.index }); if (!closed) return { id: command.id, ok: false, errorCode: 'runtime_command_failed', error: 'Tab not found' }; return { id: command.id, ok: true, data: { closed } }; } @@ -192,7 +209,7 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command: } { const lease = await manager.bindPage({ - profileId: commandProfileId(command), + profileId: commandProfileId(manager, command), session: command.session, surface: command.surface, siteSession: command.siteSession, diff --git a/src/browser/runtime/local-cloak/session-manager.test.ts b/src/browser/runtime/local-cloak/session-manager.test.ts index 4b241e1..0093536 100644 --- a/src/browser/runtime/local-cloak/session-manager.test.ts +++ b/src/browser/runtime/local-cloak/session-manager.test.ts @@ -1,5 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; +import path from 'node:path'; import { CloakSessionManager } from './session-manager.js'; +import { dispatchCloakAction } from './actions.js'; function fakeContext() { const page = { @@ -22,6 +24,10 @@ function fakeContext() { }; } +function expectedProfileDir(profileId: string): string { + return path.join('/tmp/webcmd-test', 'cloak', 'profiles', profileId); +} + describe('CloakSessionManager', () => { afterEach(() => { vi.useRealTimers(); @@ -104,4 +110,96 @@ describe('CloakSessionManager', () => { expect(lease.page.close).not.toHaveBeenCalled(); expect(await manager.listPages({ profileId: 'default' })).toHaveLength(1); }); + + it('launches a preferred profile when no Cloak profile is active', async () => { + const launched = fakeContext(); + const launchPersistentContext = vi.fn().mockResolvedValue(launched.context); + const manager = new CloakSessionManager({ + baseDir: '/tmp/webcmd-test', + launchPersistentContext, + }); + + await dispatchCloakAction(manager, { + id: 'cmd-preferred', + action: 'navigate', + session: 'work', + surface: 'browser', + url: 'https://example.com/', + preferredContextId: 'profile-default', + }); + + expect(launchPersistentContext).toHaveBeenCalledTimes(1); + expect(launchPersistentContext.mock.calls[0][0].userDataDir).toBe(expectedProfileDir('profile-default')); + }); + + it('falls back to the only active profile when the preferred profile is stale', async () => { + const launched = fakeContext(); + const launchPersistentContext = vi.fn().mockResolvedValue(launched.context); + const manager = new CloakSessionManager({ + baseDir: '/tmp/webcmd-test', + launchPersistentContext, + }); + + await dispatchCloakAction(manager, { + id: 'cmd-active', + action: 'navigate', + session: 'work', + surface: 'browser', + url: 'https://example.com/', + contextId: 'active', + }); + await dispatchCloakAction(manager, { + id: 'cmd-stale-default', + action: 'navigate', + session: 'work', + surface: 'browser', + url: 'https://example.com/next', + preferredContextId: 'stale-default', + }); + + expect(launchPersistentContext).toHaveBeenCalledTimes(1); + expect(launchPersistentContext.mock.calls[0][0].userDataDir).toBe(expectedProfileDir('active')); + }); + + it('asks for an explicit profile when a stale preferred profile meets multiple active profiles', async () => { + const launched = fakeContext(); + const launchPersistentContext = vi.fn().mockResolvedValue(launched.context); + const manager = new CloakSessionManager({ + baseDir: '/tmp/webcmd-test', + launchPersistentContext, + }); + + await dispatchCloakAction(manager, { + id: 'cmd-a', + action: 'navigate', + session: 'work-a', + surface: 'browser', + url: 'https://example.com/a', + contextId: 'profile-a', + }); + await dispatchCloakAction(manager, { + id: 'cmd-b', + action: 'navigate', + session: 'work-b', + surface: 'browser', + url: 'https://example.com/b', + contextId: 'profile-b', + }); + + const result = await dispatchCloakAction(manager, { + id: 'cmd-stale', + action: 'navigate', + session: 'work', + surface: 'browser', + url: 'https://example.com/', + preferredContextId: 'stale-default', + }); + + expect(result).toMatchObject({ + id: 'cmd-stale', + ok: false, + errorCode: 'profile_required', + }); + expect(launchPersistentContext).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/browser/runtime/local-cloak/session-manager.ts b/src/browser/runtime/local-cloak/session-manager.ts index ab0a72c..2a5d2c1 100644 --- a/src/browser/runtime/local-cloak/session-manager.ts +++ b/src/browser/runtime/local-cloak/session-manager.ts @@ -91,6 +91,10 @@ export class CloakSessionManager { })); } + activeProfileIds(): string[] { + return [...this.profiles.keys()]; + } + async getPage(input: SessionKeyInput): Promise { const profileId = normalizeProfileId(input.profileId); const session = requireSession(input.session); diff --git a/src/cli.ts b/src/cli.ts index d93aad9..fff7f28 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -37,7 +37,7 @@ import { daemonRestart, daemonStatus, daemonStop } from './commands/daemon.js'; import { log } from './logger.js'; import { bindTab, BrowserCommandError, sendCommand } from './browser/daemon-client.js'; import { fetchDaemonStatus } from './browser/daemon-transport.js'; -import { aliasForContextId, loadProfileConfig, renameProfile, resolveProfileContextId, setDefaultProfile } from './browser/profile.js'; +import { aliasForContextId, loadProfileConfig, profileRouteParams, renameProfile, resolveProfileSelection, setDefaultProfile, type ProfileSelection } from './browser/profile.js'; import { formatDaemonVersion, isDaemonStale } from './browser/daemon-version.js'; import { DEFAULT_BROWSER_CONNECT_TIMEOUT } from './browser/config.js'; import { CLI_COMMAND } from './brand.js'; @@ -520,7 +520,7 @@ async function resolveStoredBrowserTarget(page: import('./types.js').IPage, scop async function getBrowserPage( session: string, targetPage?: string, - contextId?: string, + profileSelection?: ProfileSelection, opts: { windowMode?: BrowserWindowMode } = {}, ): Promise { const { BrowserBridge } = await import('./browser/index.js'); @@ -532,11 +532,11 @@ async function getBrowserPage( timeout: DEFAULT_BROWSER_CONNECT_TIMEOUT, session, surface: 'browser', - ...(contextId && { contextId }), + ...profileRouteParams(profileSelection), ...(idleTimeout && idleTimeout > 0 && { idleTimeout }), windowMode: opts.windowMode ?? getBrowserWindowMode(undefined, 'foreground'), }); - const targetScope = getBrowserScope(session, contextId); + const targetScope = getBrowserScope(session, profileSelection?.contextId); const resolvedTargetPage = targetPage ? await resolveBrowserTargetInSession(page, targetPage, { scope: targetScope, source: 'explicit' }) : await resolveStoredBrowserTarget(page, targetScope); @@ -592,9 +592,9 @@ function getBrowserSession(command?: Command): string { throw new Error(' is a required positional argument: webcmd browser '); } -function getBrowserContextId(command?: Command): string | undefined { +function getBrowserProfileSelection(command?: Command): ProfileSelection | undefined { const raw = getCommandOption(command, 'profile'); - return resolveProfileContextId(typeof raw === 'string' && raw.trim() ? raw.trim() : undefined); + return resolveProfileSelection(typeof raw === 'string' && raw.trim() ? raw.trim() : undefined); } function getPageSession(page: import('./types.js').IPage): string { @@ -604,8 +604,11 @@ function getPageSession(page: import('./types.js').IPage): string { } function getPageScope(page: import('./types.js').IPage): string { - const contextId = (page as unknown as { contextId?: unknown }).contextId; - return getBrowserScope(getPageSession(page), typeof contextId === 'string' && contextId.trim() ? contextId.trim() : undefined); + const { contextId, preferredContextId } = page as unknown as { contextId?: unknown; preferredContextId?: unknown }; + const selected = typeof contextId === 'string' && contextId.trim() + ? contextId.trim() + : (typeof preferredContextId === 'string' && preferredContextId.trim() ? preferredContextId.trim() : undefined); + return getBrowserScope(getPageSession(page), selected); } type SnapshotSource = 'dom' | 'ax'; @@ -1007,9 +1010,9 @@ Examples: const command = args.at(-1) instanceof Command ? args.at(-1) as Command : undefined; const targetPage = getBrowserTargetId(command); const session = getBrowserSession(command); - const contextId = getBrowserContextId(command); + const profileSelection = getBrowserProfileSelection(command); const windowMode = getBrowserWindowMode(command, 'foreground'); - page = await getBrowserPage(session, targetPage, contextId, { windowMode }); + page = await getBrowserPage(session, targetPage, profileSelection, { windowMode }); await fn(page, ...args); } catch (err) { if (err instanceof BrowserConnectError) { @@ -1046,6 +1049,7 @@ Examples: type BrowserSessionCommandContext = { session: string; contextId?: string; + routing: { contextId?: string; preferredContextId?: string }; }; function browserSessionCommandAction(fn: (ctx: BrowserSessionCommandContext, opts: Record) => Promise) { @@ -1055,12 +1059,14 @@ Examples: ? optsOrCommand as Record : {}; const session = getBrowserSession(command); - const contextId = getBrowserContextId(command); + const profileSelection = getBrowserProfileSelection(command); + const contextId = profileSelection?.contextId; + const routing = profileRouteParams(profileSelection); try { const { BrowserBridge } = await import('./browser/index.js'); const bridge = new BrowserBridge(); - await bridge.connect({ timeout: DEFAULT_BROWSER_CONNECT_TIMEOUT, session, surface: 'browser', ...(contextId && { contextId }) }); - await fn({ session, contextId }, opts); + await bridge.connect({ timeout: DEFAULT_BROWSER_CONNECT_TIMEOUT, session, surface: 'browser', ...routing }); + await fn({ session, contextId, routing }, opts); } catch (err) { if (err instanceof BrowserCommandError) { emitBrowserCommandErrorEnvelope(err); @@ -1077,7 +1083,7 @@ Examples: .description('Bind an existing Cloak runtime tab to the browser session named by ') .option('--page ', 'Cloak tab page id from `webcmd browser tab list`') .option('--index ', 'Cloak tab index from `webcmd browser tab list`') - .action(browserSessionCommandAction(async ({ session, contextId }, opts) => { + .action(browserSessionCommandAction(async ({ session, contextId, routing }, opts) => { const page = typeof opts.page === 'string' && opts.page.trim() ? opts.page.trim() : undefined; const rawIndex = typeof opts.index === 'string' && opts.index.trim() ? opts.index.trim() : undefined; if ((page && rawIndex) || (!page && !rawIndex)) { @@ -1091,15 +1097,15 @@ Examples: throw new BrowserCommandError('--index must be a non-negative integer.', 'invalid_request'); } const index = rawIndex === undefined ? undefined : Number.parseInt(rawIndex, 10); - const data = await bindTab(session, { ...(contextId && { contextId }), ...(page && { page }), ...(index !== undefined && { index }) }); + const data = await bindTab(session, { ...routing, ...(page && { page }), ...(index !== undefined && { index }) }); saveBrowserTargetState(undefined, getBrowserScope(session, contextId)); console.log(JSON.stringify({ session, ...((data && typeof data === 'object') ? data as Record : { data }) }, null, 2)); })); browser.command('unbind') .description('Compatibility command; release the Cloak browser session named by ') - .action(browserSessionCommandAction(async ({ session, contextId }) => { - await sendCommand('close-window', { session, surface: 'browser', ...(contextId && { contextId }) }); + .action(browserSessionCommandAction(async ({ session, contextId, routing }) => { + await sendCommand('close-window', { session, surface: 'browser', ...routing }); saveBrowserTargetState(undefined, getBrowserScope(session, contextId)); console.log(JSON.stringify({ unbound: true, session }, null, 2)); })); diff --git a/src/constants.ts b/src/constants.ts index c61a699..9227c77 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -2,19 +2,11 @@ * Shared constants used across explore, synthesize, and pipeline modules. */ -import { CLI_COMMAND, DAEMON_HEADER_NAME, ENV_PREFIX, PRODUCT_DISPLAY_NAME } from './brand.js'; +import { DAEMON_HEADER_NAME } from './brand.js'; -/** Default daemon port for HTTP/WebSocket communication with browser extension */ +/** Default daemon port for HTTP communication with the browser runtime. */ export const DEFAULT_DAEMON_PORT = 9777; -export function unsupportedDaemonPortEnvMessage(value?: string): string { - const envName = `${ENV_PREFIX}_DAEMON_PORT`; - const suffix = value ? ` (received ${value})` : ''; - return `${envName} is no longer supported${suffix}. ` + - `The ${PRODUCT_DISPLAY_NAME} Chrome extension can only connect to localhost:${DEFAULT_DAEMON_PORT}. ` + - `Unset ${envName} and rerun ${CLI_COMMAND}.`; -} - export { DAEMON_HEADER_NAME }; /** URL query params that are volatile/ephemeral and should be stripped from patterns */ diff --git a/src/daemon.ts b/src/daemon.ts index 70dbc0d..f7c2531 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -1,15 +1,10 @@ -import { DEFAULT_DAEMON_PORT, unsupportedDaemonPortEnvMessage } from './constants.js'; +import { DEFAULT_DAEMON_PORT } from './constants.js'; import { EXIT_CODES } from './errors.js'; import { log } from './logger.js'; import { PKG_VERSION } from './version.js'; import { createDaemonServer } from './daemon/server.js'; import { LocalCloakRuntimeProvider } from './browser/runtime/local-cloak/provider.js'; -if (process.env.WEBCMD_DAEMON_PORT) { - log.error(unsupportedDaemonPortEnvMessage(process.env.WEBCMD_DAEMON_PORT)); - process.exit(EXIT_CODES.USAGE_ERROR); -} - const provider = new LocalCloakRuntimeProvider(); const daemon = createDaemonServer(provider, { port: DEFAULT_DAEMON_PORT, host: '127.0.0.1', version: PKG_VERSION }); diff --git a/src/doctor.test.ts b/src/doctor.test.ts index 2e511dd..6a5bba0 100644 --- a/src/doctor.test.ts +++ b/src/doctor.test.ts @@ -193,6 +193,38 @@ describe('doctor report rendering', () => { ])); }); + it('reports a stale default Cloak profile when it is not active', async () => { + const fs = await import('node:fs'); + const os = await import('node:os'); + const path = await import('node:path'); + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'webcmd-doctor-profile-')); + fs.writeFileSync( + path.join(configDir, 'browser-profiles.json'), + JSON.stringify({ version: 1, aliases: { work: 'profile-default' }, defaultContextId: 'profile-default' }), + ); + vi.stubEnv('WEBCMD_CONFIG_DIR', configDir); + try { + mockGetDaemonHealth.mockResolvedValueOnce({ + state: 'ready', + status: { + runtimeConnected: true, + runtimeName: 'Cloak', + profiles: [{ contextId: 'active-profile', runtimeConnected: true, pending: 0 }], + }, + }); + + const report = await runBrowserDoctor(); + + expect(report.issues).toEqual(expect.arrayContaining([ + expect.stringContaining('Default Cloak profile is not active: work (profile-default)'), + ])); + expect(report.issues.join('\n')).toContain('fall back to the only active profile: active-profile'); + } finally { + vi.unstubAllEnvs(); + fs.rmSync(configDir, { recursive: true, force: true }); + } + }); + it('reports flapping when live check succeeds but final status shows runtime disconnected', async () => { mockGetDaemonHealth.mockResolvedValueOnce({ state: 'no-runtime', status: { runtimeConnected: false, runtimeName: 'Cloak' } }); diff --git a/src/doctor.ts b/src/doctor.ts index 20482d5..4a38329 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -131,6 +131,20 @@ export async function runBrowserDoctor(opts: DoctorOptions = {}): Promise p.contextId === staleDefault)) { + const alias = aliasForContextId(profileConfig, staleDefault); + const label = alias ? `${alias} (${staleDefault})` : staleDefault; + const fallbackNote = profiles.length === 1 + ? `Commands currently fall back to the only active profile: ${profiles[0].contextId}.` + : 'Multiple profiles are active, so commands will ask you to choose.'; + issues.push( + `Default Cloak profile is not active: ${label}.\n` + + ` ${fallbackNote}\n` + + ' Refresh it with: webcmd profile list, then webcmd profile use .', + ); + } if (adapterShadows.length > 0) { issues.push(formatAdapterShadowIssue(adapterShadows)); } diff --git a/src/execution.ts b/src/execution.ts index dfba3bd..2c2691e 100644 --- a/src/execution.ts +++ b/src/execution.ts @@ -29,7 +29,7 @@ import { executePipeline } from './pipeline/index.js'; import { adapterLoadError, ArgumentError, CommandExecutionError, attachTraceReceipt, getErrorMessage } from './errors.js'; import { shouldUseBrowserSession } from './capabilityRouting.js'; import { getBrowserFactory, browserSession, runWithTimeout, DEFAULT_BROWSER_COMMAND_TIMEOUT, type BrowserWindowMode } from './runtime.js'; -import { resolveProfileContextId } from './browser/profile.js'; +import { profileRouteParams, resolveProfileSelection } from './browser/profile.js'; import { setDaemonCommandTimeoutSeconds } from './browser/daemon-client.js'; import { emitHook, type HookContext } from './hooks.js'; import { log } from './logger.js'; @@ -251,7 +251,9 @@ export async function executeCommand( } const BrowserFactory = getBrowserFactory(cmd.site); - const contextId = resolveProfileContextId(opts.profile); + const profileSelection = resolveProfileSelection(opts.profile); + const profileRouting = profileRouteParams(profileSelection); + const contextId = profileSelection?.contextId; const internal = cmd as InternalCliCommand; const siteSession = resolveSiteSession(cmd, opts.siteSession); const session = resolveAdapterBrowserSession(cmd, siteSession); @@ -371,7 +373,7 @@ export async function executeCommand( if (!keepTab) await page.closeWindow?.().catch(() => {}); throw err; } - }, { session, cdpEndpoint, contextId, windowMode, surface: 'adapter', siteSession }); + }, { session, cdpEndpoint, ...profileRouting, windowMode, surface: 'adapter', siteSession }); } else { // Non-browser commands: enforce a timeout only when the command exposes // a `--timeout` arg (and the resolved value is positive). Without that diff --git a/src/external.test.ts b/src/external.test.ts index 568cd2e..0f926b2 100644 --- a/src/external.test.ts +++ b/src/external.test.ts @@ -22,7 +22,8 @@ vi.mock('node:os', async () => { }; }); -import { formatExternalCliLabel, installExternalCli, parseCommand, type ExternalCliConfig } from './external.js'; +import { spawnSync } from 'node:child_process'; +import { executeExternalCli, formatExternalCliLabel, installExternalCli, parseCommand, type ExternalCliConfig } from './external.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -130,3 +131,49 @@ describe('installExternalCli', () => { expect(mockExecFileSync).toHaveBeenCalledTimes(1); }); }); + +describe('executeExternalCli passthrough', () => { + const spawnMock = vi.mocked(spawnSync); + const cli: ExternalCliConfig = { name: 'tg', binary: 'tg', description: '' }; + + beforeEach(() => { + spawnMock.mockReset(); + mockExecFileSync.mockReset(); + mockExecFileSync.mockReturnValue(Buffer.from('')); + mockPlatform.mockReturnValue('darwin'); + process.exitCode = undefined; + }); + + it('retries through the shell on Windows when the binary is a .cmd shim', () => { + mockPlatform.mockReturnValue('win32'); + const einval = Object.assign(new Error('spawnSync tg EINVAL'), { code: 'EINVAL' }); + spawnMock + .mockReturnValueOnce({ error: einval, status: null, signal: null } as unknown as ReturnType) + .mockReturnValueOnce({ status: 0, signal: null } as unknown as ReturnType); + + executeExternalCli('tg', ['send', 'hello world', '--to', 'a"b'], [cli]); + + expect(spawnMock).toHaveBeenCalledTimes(2); + expect(spawnMock).toHaveBeenNthCalledWith(1, 'tg', ['send', 'hello world', '--to', 'a"b'], { stdio: 'inherit' }); + expect(spawnMock).toHaveBeenNthCalledWith(2, 'tg send "hello world" --to "a""b"', { stdio: 'inherit', shell: true }); + expect(process.exitCode).toBe(0); + }); + + it('does not retry through the shell on non-Windows platforms', () => { + const einval = Object.assign(new Error('spawnSync tg EINVAL'), { code: 'EINVAL' }); + spawnMock.mockReturnValueOnce({ error: einval, status: null, signal: null } as unknown as ReturnType); + + executeExternalCli('tg', [], [cli]); + + expect(spawnMock).toHaveBeenCalledTimes(1); + expect(process.exitCode).toBe(1); + }); + + it('reports a non-zero exit code when the child dies from a signal', () => { + spawnMock.mockReturnValueOnce({ status: null, signal: 'SIGKILL' } as unknown as ReturnType); + + executeExternalCli('tg', [], [cli]); + + expect(process.exitCode).toBe(1); + }); +}); diff --git a/src/external.ts b/src/external.ts index ddfc635..9f95d7b 100644 --- a/src/external.ts +++ b/src/external.ts @@ -197,18 +197,38 @@ export function executeExternalCli(name: string, args: string[], preloaded?: Ext } // 3. Passthrough execution with stdio inherited - const result = spawnSync(cli.binary, args, { stdio: 'inherit' }); + const result = spawnPassthrough(cli.binary, args); if (result.error) { log.error(`Failed to execute '${cli.binary}': ${result.error.message}`); process.exitCode = EXIT_CODES.GENERIC_ERROR; return; } - + + if (result.signal) { + process.exitCode = EXIT_CODES.GENERIC_ERROR; + return; + } + if (result.status !== null) { process.exitCode = result.status; } } +function quoteForCmdShell(token: string): string { + if (token !== '' && !/[\s"^&|<>%()]/.test(token)) return token; + return `"${token.replace(/"/g, '""')}"`; +} + +function spawnPassthrough(binary: string, args: string[]): ReturnType { + const direct = spawnSync(binary, args, { stdio: 'inherit' }); + const errorCode = (direct.error as NodeJS.ErrnoException | undefined)?.code; + if (os.platform() === 'win32' && (errorCode === 'EINVAL' || errorCode === 'ENOENT')) { + const command = [binary, ...args].map(quoteForCmdShell).join(' '); + return spawnSync(command, { stdio: 'inherit', shell: true }); + } + return direct; +} + export interface RegisterOptions { binary?: string; install?: string; diff --git a/src/main.ts b/src/main.ts index ac42f9f..0b9bcb3 100644 --- a/src/main.ts +++ b/src/main.ts @@ -22,7 +22,6 @@ import { findPackageRoot, getCliManifestPath } from './package-paths.js'; import { PKG_VERSION } from './version.js'; import { EXIT_CODES } from './errors.js'; import { isSupportedNodeVersion, MIN_SUPPORTED_NODE_MAJOR } from './runtime-detect.js'; -import { unsupportedDaemonPortEnvMessage } from './constants.js'; import { CONFIG_DIR_NAME } from './brand.js'; const __filename = fileURLToPath(import.meta.url); @@ -48,11 +47,6 @@ if (typeof (globalThis as { Bun?: unknown }).Bun === 'undefined' && !isSupported process.exit(EXIT_CODES.CONFIG_ERROR); } -if (process.env.WEBCMD_DAEMON_PORT) { - process.stderr.write(`error: ${unsupportedDaemonPortEnvMessage(process.env.WEBCMD_DAEMON_PORT)}\n`); - process.exit(EXIT_CODES.CONFIG_ERROR); -} - // Fast path: --version (only when it's the top-level intent, not passed to a subcommand) // e.g. `webcmd --version` or `webcmd -V`, but NOT `webcmd gh --version` if (argv[0] === '--version' || argv[0] === '-V') { diff --git a/src/runtime-identity.test.ts b/src/runtime-identity.test.ts index 292b969..3a874ea 100644 --- a/src/runtime-identity.test.ts +++ b/src/runtime-identity.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from 'vitest'; import * as path from 'node:path'; -import { unsupportedDaemonPortEnvMessage } from './constants.js'; import { getUserWebcmdDir, getUserClisDir, getPluginsDir } from './discovery.js'; describe('webcmd runtime identity', () => { @@ -9,12 +8,4 @@ describe('webcmd runtime identity', () => { expect(getUserClisDir('/home/tester')).toBe(path.join('/home/tester', '.webcmd', 'clis')); expect(getPluginsDir('/home/tester')).toBe(path.join('/home/tester', '.webcmd', 'plugins')); }); - - it('reports unsupported daemon port with WEBCMD env names', () => { - const legacyEnvName = ['OPEN', 'CLI_DAEMON_PORT'].join(''); - expect(unsupportedDaemonPortEnvMessage('1234')).toContain('WEBCMD_DAEMON_PORT'); - expect(unsupportedDaemonPortEnvMessage('1234')).toContain('Webcmd'); - expect(unsupportedDaemonPortEnvMessage('1234')).toContain('rerun webcmd'); - expect(unsupportedDaemonPortEnvMessage('1234')).not.toContain(legacyEnvName); - }); }); diff --git a/src/runtime.ts b/src/runtime.ts index b0ec319..1e1c229 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -54,14 +54,14 @@ export function withTimeoutMs( /** Interface for browser factory (BrowserBridge or test mocks) */ export interface IBrowserFactory { - connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent' }): Promise; + connect(opts?: { timeout?: number; session?: string; cdpEndpoint?: string; contextId?: string; preferredContextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent' }): Promise; close(): Promise; } export async function browserSession( BrowserFactory: new () => IBrowserFactory, fn: (page: IPage) => Promise, - opts: { session?: string; cdpEndpoint?: string; contextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent' } = {}, + opts: { session?: string; cdpEndpoint?: string; contextId?: string; preferredContextId?: string; idleTimeout?: number; windowMode?: BrowserWindowMode; surface?: BrowserSurface; siteSession?: 'ephemeral' | 'persistent' } = {}, ): Promise { const browser = new BrowserFactory(); try { @@ -70,6 +70,7 @@ export async function browserSession( session: opts.session, cdpEndpoint: opts.cdpEndpoint, contextId: opts.contextId, + preferredContextId: opts.preferredContextId, idleTimeout: opts.idleTimeout, windowMode: opts.windowMode, surface: opts.surface, diff --git a/tests/e2e/helpers.ts b/tests/e2e/helpers.ts index 3cf8210..4bd24ae 100644 --- a/tests/e2e/helpers.ts +++ b/tests/e2e/helpers.ts @@ -36,7 +36,10 @@ export async function runCli( args: string[], opts: { timeout?: number; env?: Record; maxBuffer?: number } = {}, ): Promise { - const timeout = opts.timeout ?? 30_000; + // Keep the child timeout below the common 30s Vitest timeout so flaky + // commands return a structured non-zero result instead of killing the test + // at the framework layer. + const timeout = opts.timeout ?? 25_000; const maxBuffer = opts.maxBuffer ?? DEFAULT_MAX_BUFFER_BYTES; try { const runtime = process.env.WEBCMD_TEST_RUNTIME || 'node';