From d1c7e00995ed80f26a1b969afbda8e983326d33c Mon Sep 17 00:00:00 2001 From: Agnik47 <140933190+Agnik47@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:30:28 +0530 Subject: [PATCH] fix: make -v effective in hosted, browser, and auth commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `-v/--verbose` was advertised more widely than it worked. Hosted dispatch parsed the flag and dropped it, and the raw browser leaves and auth probes rejected it outright even though the diagnostics behind it already existed. Hosted mode was the actual no-op: `parseCommandSurface` produced `parsed.verbose`, but `dispatchHosted` passed only format/trace/profile/session to `client.execute`, so `webcmd -v` did nothing in the cloud while the same flag worked locally. There were also no verbose diagnostics in the hosted client at all, so forwarding alone would still have shown nothing. - Add `enableVerbose()` to the logger so every entry point applies the one `WEBCMD_VERBOSE` contract `isVerbose()` already reads. Passing `false` leaves the environment alone, so an exported `WEBCMD_VERBOSE=1` is not cancelled by a flagless invocation. - Apply it in `dispatchHosted`, and give `HostedClient.authenticatedFetch` request/response/failure diagnostics so hosted `-v` is observable. These carry method, path, status and elapsed time only — never the bearer token, the workspace header, or bodies. - Accept `-v` on the bridge/CDP browser leaves (`tabs`, `bind`, `run`, `snapshot`, `close`), which enables the diagnostics `cdp.ts` already gates on `isVerbose()`. Filesystem-only leaves (`init`, `fork`) keep no flag rather than advertising a no-op. - Accept `-v` on `auth status` and `auth refresh`, whose probes drive that same browser/daemon stack. The browser flag is declared in both `cli.ts` and `browserCommandCatalog` because local help comes from Commander while hosted help is rendered from the catalog, and the two are asserted byte-identical; `browserOptionFlags` grows a `verbose` case so hosted renders the `-v` short form. `-v` stays a local concern: it is deliberately not added to the `/v1/execute` request body, and a test pins that. Refs #174 --- src/browser/command-catalog.test.ts | 30 +++++++++- src/browser/command-catalog.ts | 33 ++++++++++- src/cli.test.ts | 61 ++++++++++++++++++- src/cli.ts | 38 ++++++++---- src/commands/auth.test.ts | 32 +++++++++- src/commands/auth.ts | 7 +++ src/hosted/client.test.ts | 90 ++++++++++++++++++++++++++++- src/hosted/client.ts | 43 +++++++++----- src/hosted/runner.test.ts | 72 ++++++++++++++++++++++- src/hosted/runner.ts | 6 ++ src/logger.test.ts | 39 ++++++++++++- src/logger.ts | 17 ++++++ 12 files changed, 434 insertions(+), 34 deletions(-) diff --git a/src/browser/command-catalog.test.ts b/src/browser/command-catalog.test.ts index 5d5f468d..702172ce 100644 --- a/src/browser/command-catalog.test.ts +++ b/src/browser/command-catalog.test.ts @@ -1,7 +1,7 @@ import type { Command } from 'commander'; import { describe, expect, it } from 'vitest'; import { createProgram } from '../cli.js'; -import { browserCommandCatalog, browserOptionValueParser } from './command-catalog.js'; +import { browserCommandCatalog, browserOptionFlags, browserOptionValueParser } from './command-catalog.js'; function browserCommand(): Command { const browser = createProgram('', '').commands.find(command => command.name() === 'browser'); @@ -66,6 +66,7 @@ describe('browserCommandCatalog', () => { const commands = new Map(browserCommandCatalog.map(command => [command.command, command])); expect(commands.get('bind')?.options).toEqual([ expect.objectContaining({ name: 'page', required: true }), + expect.objectContaining({ name: 'verbose', type: 'boolean' }), ]); expect(commands.get('run')?.options.map(option => option.name)).toEqual([ 'stdin', @@ -74,13 +75,38 @@ describe('browserCommandCatalog', () => { 'maxOutput', 'snapshotMode', 'noSnapshotDiff', + 'verbose', ]); }); it('includes snapshot as the read-only browser inspection command', () => { const snapshot = browserCommandCatalog.find(command => command.command === 'snapshot'); expect(snapshot).toMatchObject({ action: 'snapshot', sessionPolicy: 'require-existing' }); - expect(snapshot?.options.map(option => option.name)).toEqual(['snapshotMode', 'ref', 'maxOutput']); + expect(snapshot?.options.map(option => option.name)).toEqual(['snapshotMode', 'ref', 'maxOutput', 'verbose']); + }); + + // Verbose belongs to the bridge/CDP leaves only: the diagnostics behind it live + // in the CDP client, so a filesystem-only leaf would be advertising a no-op (#174). + it('exposes verbose on bridge leaves and withholds it from filesystem-only ones', () => { + const optionNames = (name: string) => browserCommandCatalog + .find(command => command.command === name) + ?.options.map(option => option.name) ?? []; + + for (const leaf of ['tabs', 'bind', 'run', 'snapshot', 'close']) { + expect(optionNames(leaf)).toContain('verbose'); + } + for (const leaf of ['init', 'fork', 'verify']) { + expect(optionNames(leaf)).not.toContain('verbose'); + } + }); + + it('renders the verbose flag with its local short form', () => { + const verbose = browserCommandCatalog + .find(command => command.command === 'tabs') + ?.options.find(option => option.name === 'verbose'); + + expect(verbose).toBeDefined(); + expect(browserOptionFlags(verbose!, 'tabs')).toBe('-v, --verbose'); }); it('parses run snapshot mode as act or tree only', () => { diff --git a/src/browser/command-catalog.ts b/src/browser/command-catalog.ts index a2222a7f..3d393ff5 100644 --- a/src/browser/command-catalog.ts +++ b/src/browser/command-catalog.ts @@ -31,6 +31,27 @@ function flag(name: string, description: string): HostedArgumentContract { return { name, type: 'boolean', description, positional: false, required: false, variadic: false }; } +/** + * `-v` for a browser leaf that performs bridge/CDP I/O, whose diagnostics are + * already gated on `isVerbose()` (`src/browser/cdp.ts`). + * + * Declared here as well as in `cli.ts` because hosted help is rendered from this + * catalog while local help comes from Commander, and the two are asserted to be + * byte-identical. The explicit `false` default reproduces local's + * "(default: false)" suffix. + */ +function verboseFlag(): HostedArgumentContract { + return { + name: 'verbose', + type: 'boolean', + description: 'Debug output', + positional: false, + required: false, + variadic: false, + default: false, + }; +} + function command( commandPath: string, description: string, @@ -78,6 +99,7 @@ const adapterNamePositional: HostedArgumentContract = { /** Exact local Commander flags for every catalogued browser option. */ export function browserOptionFlags(option: HostedArgumentContract, commandPath?: string): string { const longName = option.name.replace(/[A-Z]/g, character => `-${character.toLowerCase()}`); + if (option.name === 'verbose') return '-v, --verbose'; if (option.type === 'boolean') return `--${longName}`; const valueName = option.name === 'page' ? 'id' : option.name === 'file' ? 'path' @@ -119,10 +141,13 @@ export function browserOptionValueParser( } export const browserCommandCatalog: readonly HostedBrowserCommandContract[] = [ - command('tabs', 'List pages in the existing browser session', 'tabs', [], [], 'require-existing'), + command('tabs', 'List pages in the existing browser session', 'tabs', [], [ + verboseFlag(), + ], 'require-existing'), command('init', 'Generate an adapter scaffold', 'init', [adapterNamePositional], [], 'create-or-reuse'), command('bind', 'Bind this session to an existing page', 'bind', [], [ option('page', 'Stable page id returned by tabs', { required: true }), + verboseFlag(), ], 'require-existing'), command('fork', 'Fork an installed plugin command into a private copy', 'fork', [{ name: 'name', @@ -148,11 +173,15 @@ export const browserCommandCatalog: readonly HostedBrowserCommandContract[] = [ option('maxOutput', 'Maximum returned characters'), option('snapshotMode', 'Snapshot mode for automatic diff: act or tree', { default: 'act' }), flag('noSnapshotDiff', 'Skip the automatic before/after snapshot diff'), + verboseFlag(), ], 'create-or-reuse'), command('snapshot', 'Inspect the current page with a compact accessibility snapshot', 'snapshot', [], [ option('snapshotMode', 'Snapshot mode: act, tree, or read', { default: 'act' }), option('ref', 'Render only the subtree rooted at this snapshot ref'), option('maxOutput', 'Maximum returned characters'), + verboseFlag(), ], 'require-existing'), - command('close', 'Close or detach this browser session', 'close-window', [], [], 'close-existing'), + command('close', 'Close or detach this browser session', 'close-window', [], [ + verboseFlag(), + ], 'close-existing'), ] as const; diff --git a/src/cli.test.ts b/src/cli.test.ts index b4831fca..52f97a6f 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1175,7 +1175,7 @@ name: 'search', usage: 'webcmd browser bind [options]', positionals: [], }); - expect(bind.command_options.map((option: any) => option.name)).toEqual(['page']); + expect(bind.command_options.map((option: any) => option.name)).toEqual(['page', 'verbose']); expect(data.structured_help).toMatchObject({ formats: ['yaml', 'json'], usage: 'webcmd browser --help -f yaml', @@ -1797,6 +1797,65 @@ describe('browser raw session commands', () => { }); }); + // The CDP client already gates diagnostics on isVerbose(), but the raw browser + // leaves rejected -v, so those diagnostics were unreachable from the CLI (#174). + describe('raw browser verbose flag', () => { + afterEach(() => { + delete process.env.WEBCMD_VERBOSE; + }); + + it.each(['tabs', 'snapshot', 'close'])('accepts -v on browser %s', async (leaf) => { + delete process.env.WEBCMD_VERBOSE; + const program = createProgram('', ''); + + await program.parseAsync(['node', 'webcmd', '--session', 'session_test', 'browser', leaf, '-v']); + + expect(process.exitCode).toBeUndefined(); + expect(process.env.WEBCMD_VERBOSE).toBe('1'); + }); + + it('accepts -v on browser bind', async () => { + delete process.env.WEBCMD_VERBOSE; + const program = createProgram('', ''); + + await program.parseAsync(['node', 'webcmd', '--session', 'session_test', 'browser', 'bind', '--page', 'page-123', '-v']); + + expect(process.env.WEBCMD_VERBOSE).toBe('1'); + expect(mockSendCommand).toHaveBeenCalledWith('bind', { + session: 'session_test', surface: 'browser', page: 'page-123', + }); + }); + + it('leaves verbose mode off when the flag is absent', async () => { + delete process.env.WEBCMD_VERBOSE; + const program = createProgram('', ''); + + await program.parseAsync(['node', 'webcmd', '--session', 'session_test', 'browser', 'tabs']); + + expect(process.env.WEBCMD_VERBOSE).toBeUndefined(); + }); + + // Structural check so `run` is covered too: it needs a program source, so it + // cannot reach the action body from argv alone. + // + // Filesystem-only leaves stay without the flag: there are no diagnostics + // behind it, and advertising one would be the no-op #174 set out to remove. + it('declares the flag on bridge leaves and withholds it from filesystem-only ones', () => { + const program = createProgram('', ''); + const browser = program.commands.find(command => command.name() === 'browser'); + const flagsFor = (name: string) => browser?.commands + .find(command => command.name() === name) + ?.options.map(option => option.long) ?? []; + + for (const leaf of ['tabs', 'bind', 'run', 'snapshot', 'close']) { + expect(flagsFor(leaf)).toContain('--verbose'); + } + for (const leaf of ['init', 'fork']) { + expect(flagsFor(leaf)).not.toContain('--verbose'); + } + }); + }); + it('binds raw browser daemon operations to one logical run', async () => { let run = getDaemonRunContext(); mockSendCommand.mockImplementation(async () => { diff --git a/src/cli.ts b/src/cli.ts index 9f645daf..79d3cd4a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -41,7 +41,7 @@ import { analyzeSite, type PageSignals } from './browser/analyze.js'; import { browserOptionValueParser } from './browser/command-catalog.js'; import { registerAuthCommands } from './commands/auth.js'; import { daemonRestart, daemonStatus, daemonStop } from './commands/daemon.js'; -import { isVerbose, log } from './logger.js'; +import { enableVerbose, isVerbose, log } from './logger.js'; import { BrowserCommandError, listExistingBrowserTabs, releaseSiteSessionLease, sendCommand } from './browser/daemon-client.js'; import { fetchDaemonStatus } from './browser/daemon-transport.js'; import { aliasForContextId, loadProfileConfig, profileListRows, profileRouteParams, renameProfile, resolveProfileSelection, setDefaultProfile, type ProfileSelection } from './browser/profile.js'; @@ -579,7 +579,20 @@ function sessionCreateOutput(data: unknown): unknown { } function applyVerbose(opts: { verbose?: boolean }): void { - if (opts.verbose) process.env.WEBCMD_VERBOSE = '1'; + enableVerbose(opts.verbose === true); +} + +/** + * Add `-v` to a browser leaf that performs bridge/CDP I/O. + * + * The CDP client already gates diagnostics on `isVerbose()` + * (`src/browser/cdp.ts`), but the raw browser leaves rejected the flag, so those + * diagnostics were only reachable by setting `WEBCMD_VERBOSE` by hand (#174). + * Filesystem-only leaves (`init`, `fork`) are deliberately left out: a flag + * there would advertise diagnostics that do not exist. + */ +function withBrowserVerbose(command: Command): Command { + return command.option('-v, --verbose', 'Debug output', false); } function formatChildCommandSummary(command: Command): string { @@ -1142,6 +1155,7 @@ cli({ function rawBrowserAction(fn: (session: string, routing: { contextId?: string; preferredContextId?: string }, opts: Record) => Promise) { return async (opts: Record, command: Command) => { + applyVerbose(opts as { verbose?: boolean }); const runId = generateRunId(); const commandName = `browser/${command.name()}`; let releaseRun = true; @@ -1172,11 +1186,11 @@ cli({ }; } - browser.addCommand(new Command('tabs') + browser.addCommand(withBrowserVerbose(new Command('tabs') .description('List pages in the existing browser session') - .action(rawBrowserAction((session, routing) => listExistingBrowserTabs(session, routing)))); + .action(rawBrowserAction((session, routing) => listExistingBrowserTabs(session, routing))))); - browser.addCommand(new Command('bind') + browser.addCommand(withBrowserVerbose(new Command('bind') .description('Bind this session to an existing page') .addOption(new Option('--page ', 'Stable page id returned by tabs') .makeOptionMandatory() @@ -1185,16 +1199,16 @@ cli({ const page = typeof opts.page === 'string' ? opts.page.trim() : ''; if (!page) throw new BrowserCommandError('--page must be a non-empty stable page id', 'invalid_request'); return sendCommand('bind', { session, surface: 'browser', ...routing, page }); - }))); + })))); - const runCommand = new Command('run') + const runCommand = withBrowserVerbose(new Command('run') .description('Run JavaScript with Playwright') .option('--stdin', 'Read the program from stdin') .option('--file ', 'Read the program from a file') .addOption(new Option('--timeout ', 'Execution timeout in seconds').argParser(browserOptionValueParser('run', 'timeout')!)) .addOption(new Option('--max-output ', 'Maximum returned characters').argParser(browserOptionValueParser('run', 'maxOutput')!)) .addOption(new Option('--snapshot-mode ', 'Snapshot mode for automatic diff: act or tree').default('act').argParser(browserOptionValueParser('run', 'snapshotMode')!)) - .option('--no-snapshot-diff', 'Skip the automatic before/after snapshot diff'); + .option('--no-snapshot-diff', 'Skip the automatic before/after snapshot diff')); runCommand.action(rawBrowserAction(async (session, routing, opts) => { let source: string; try { @@ -1218,7 +1232,7 @@ cli({ })); browser.addCommand(runCommand); - browser.addCommand(new Command('snapshot') + browser.addCommand(withBrowserVerbose(new Command('snapshot') .description('Inspect the current page with a compact accessibility snapshot') .addOption(new Option('--snapshot-mode ', 'Snapshot mode: act, tree, or read').default('act').argParser(browserOptionValueParser('snapshot', 'snapshotMode')!)) .option('--ref ', 'Render only the subtree rooted at this snapshot ref') @@ -1230,15 +1244,15 @@ cli({ snapshotMode: opts.snapshotMode === 'tree' || opts.snapshotMode === 'read' ? opts.snapshotMode : 'act', ...(typeof opts.ref === 'string' ? { ref: opts.ref } : {}), ...(typeof opts.maxOutput === 'number' ? { maxOutputChars: opts.maxOutput } : {}), - })))); + }))))); - browser.addCommand(new Command('close') + browser.addCommand(withBrowserVerbose(new Command('close') .description('Close or detach this browser session') .action(rawBrowserAction((session, routing) => sendCommand('close-window', { session, surface: 'browser', ...routing, - })))); + }))))); // ── Built-in: doctor / completion ────────────────────────────────────────── program diff --git a/src/commands/auth.test.ts b/src/commands/auth.test.ts index f68b513c..630f17d8 100644 --- a/src/commands/auth.test.ts +++ b/src/commands/auth.test.ts @@ -2,7 +2,7 @@ import { mkdtemp, readFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { Command } from 'commander'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { BrowserCliCommand } from '../registry.js'; const executeCommandMock = vi.hoisted(() => vi.fn()); @@ -307,3 +307,33 @@ describe('auth command format validation', () => { expect(executeCommandMock).not.toHaveBeenCalled(); }); }); + +// Both probes drive the browser/daemon stack, whose CDP diagnostics are already +// gated on isVerbose() — so -v has something observable behind it here (#174). +describe('auth verbose flag', () => { + afterEach(() => { + delete process.env.WEBCMD_VERBOSE; + }); + + it.each(['status', 'refresh'] as const)('turns on verbose mode for auth %s', async (subcommand) => { + delete process.env.WEBCMD_VERBOSE; + registerWhoami('alpha', { quick: true, quickLoggedIn: true }); + const program = new Command('webcmd'); + registerAuthCommands(program); + + await program.parseAsync(['node', 'webcmd', 'auth', subcommand, '--site', 'alpha', '-v']); + + expect(process.env.WEBCMD_VERBOSE).toBe('1'); + }); + + it.each(['status', 'refresh'] as const)('leaves verbose mode off for auth %s without the flag', async (subcommand) => { + delete process.env.WEBCMD_VERBOSE; + registerWhoami('alpha', { quick: true, quickLoggedIn: true }); + const program = new Command('webcmd'); + registerAuthCommands(program); + + await program.parseAsync(['node', 'webcmd', 'auth', subcommand, '--site', 'alpha']); + + expect(process.env.WEBCMD_VERBOSE).toBeUndefined(); + }); +}); diff --git a/src/commands/auth.ts b/src/commands/auth.ts index c7fb78ff..d881b725 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -6,6 +6,7 @@ import { Command, InvalidArgumentError, Option } from 'commander'; import { OUTPUT_FORMAT_HELP, resolveOutputFormat } from '../command-surface.js'; import { AuthRequiredError, CliError, getErrorMessage } from '../errors.js'; import { executeCommand } from '../execution.js'; +import { enableVerbose } from '../logger.js'; import { type BrowserCliCommand, type CliCommand, @@ -467,7 +468,11 @@ export function registerAuthCommands(program: Command): Command { .option('--timeout ', 'Per-site timeout in seconds') .addOption(new Option('--only ', 'Filter rows by status').choices(['all', 'logged-in', 'not-logged-in', 'unknown', 'error']).default('all')) .option('-f, --format ', OUTPUT_FORMAT_HELP, 'table') + .option('-v, --verbose', 'Debug output', false) .action(async (opts) => { + // Both auth probes drive the browser/daemon stack, so verbose mode surfaces + // the CDP diagnostics those layers already gate on `isVerbose()` (#174). + enableVerbose(opts.verbose === true); const fmt = resolveOutputFormat(opts.format); if (fmt === null) return; const globals = typeof status.optsWithGlobals === 'function' ? status.optsWithGlobals() as Record : {}; @@ -496,7 +501,9 @@ export function registerAuthCommands(program: Command): Command { .option('--concurrency ', 'Maximum sites to refresh at once') .option('--timeout ', 'Per-site timeout in seconds') .option('-f, --format ', OUTPUT_FORMAT_HELP, 'table') + .option('-v, --verbose', 'Debug output', false) .action(async (opts) => { + enableVerbose(opts.verbose === true); const fmt = resolveOutputFormat(opts.format); if (fmt === null) return; const globals = typeof refresh.optsWithGlobals === 'function' ? refresh.optsWithGlobals() as Record : {}; diff --git a/src/hosted/client.test.ts b/src/hosted/client.test.ts index d1ddf056..03d2eda1 100644 --- a/src/hosted/client.test.ts +++ b/src/hosted/client.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { HostedClient, HostedClientError, resolveWorkspace } from './client.js'; const invalidTraceUrlCases = [ @@ -1530,3 +1530,91 @@ describe('resolveWorkspace', () => { expect(resolveWorkspace([], {})).toBeUndefined(); }); }); + +describe('HostedClient verbose diagnostics', () => { + const SECRET_KEY = 'sk-hosted-secret-value'; + + function captureStderr(): { lines: () => string[]; restore: () => void } { + const written: string[] = []; + const spy = vi.spyOn(process.stderr, 'write').mockImplementation((chunk: unknown) => { + written.push(String(chunk)); + return true; + }); + return { lines: () => written, restore: () => spy.mockRestore() }; + } + + beforeEach(() => { + delete process.env.WEBCMD_VERBOSE; + }); + + afterEach(() => { + delete process.env.WEBCMD_VERBOSE; + vi.restoreAllMocks(); + }); + + it('stays silent when verbose mode is off', async () => { + const stderr = captureStderr(); + const client = new HostedClient({ + apiBaseUrl: 'https://api.example.com', + apiKey: SECRET_KEY, + fetchImpl: async () => new Response(JSON.stringify({ ok: true, profiles: [] })), + }); + + await client.listProfiles(); + stderr.restore(); + + expect(stderr.lines()).toEqual([]); + }); + + it('reports method, path and status when verbose mode is on', async () => { + process.env.WEBCMD_VERBOSE = '1'; + const stderr = captureStderr(); + const client = new HostedClient({ + apiBaseUrl: 'https://api.example.com', + apiKey: SECRET_KEY, + fetchImpl: async () => new Response(JSON.stringify({ ok: true, profiles: [] })), + }); + + await client.listProfiles(); + stderr.restore(); + + const output = stderr.lines().join(''); + expect(output).toContain('hosted → GET /v1/profiles'); + expect(output).toMatch(/hosted ← GET \/v1\/profiles 200 \(\d+ms\)/); + }); + + // `-v` is a debugging aid; it must never turn into a credential dump. + it('never writes the bearer token or workspace header', async () => { + process.env.WEBCMD_VERBOSE = '1'; + const stderr = captureStderr(); + const client = new HostedClient({ + apiBaseUrl: 'https://api.example.com', + apiKey: SECRET_KEY, + workspace: 'ws_secret_workspace', + fetchImpl: async () => new Response(JSON.stringify({ ok: true, profiles: [] })), + }); + + await client.listProfiles(); + stderr.restore(); + + const output = stderr.lines().join(''); + expect(output).not.toContain(SECRET_KEY); + expect(output).not.toContain('ws_secret_workspace'); + expect(output).not.toContain('Bearer'); + }); + + it('reports transport failures and still rethrows them', async () => { + process.env.WEBCMD_VERBOSE = '1'; + const stderr = captureStderr(); + const client = new HostedClient({ + apiBaseUrl: 'https://api.example.com', + apiKey: SECRET_KEY, + fetchImpl: async () => { throw new Error('ECONNREFUSED'); }, + }); + + await expect(client.listProfiles()).rejects.toThrow('ECONNREFUSED'); + stderr.restore(); + + expect(stderr.lines().join('')).toMatch(/hosted ✖ GET \/v1\/profiles failed after \d+ms: ECONNREFUSED/); + }); +}); diff --git a/src/hosted/client.ts b/src/hosted/client.ts index 7f9c54d4..b09af3f0 100644 --- a/src/hosted/client.ts +++ b/src/hosted/client.ts @@ -1,4 +1,5 @@ import { attachTraceReceipt, CliError, EXIT_CODES, type ExitCode } from '../errors.js'; +import { log } from '../logger.js'; import { HOSTED_SESSION_PROTOCOL_VERSION } from './types.js'; import type { HostedBrowserActionRequest, @@ -458,19 +459,35 @@ export class HostedClient { return body; } - private authenticatedFetch(path: string, init: RequestInit = {}): Promise { - return this.fetchImpl(`${this.apiBaseUrl}${path}`, { - ...init, - headers: { - accept: 'application/json', - authorization: `Bearer ${this.apiKey}`, - 'x-webcmd-session-protocol-version': String(HOSTED_SESSION_PROTOCOL_VERSION), - 'x-webcmd-client-capabilities': 'hosted-live-view-v1', - ...(init.body ? { 'content-type': 'application/json' } : {}), - ...(this.workspace ? { 'x-webcmd-workspace': this.workspace } : {}), - ...(init.headers ?? {}), - }, - }); + private async authenticatedFetch(path: string, init: RequestInit = {}): Promise { + // Verbose diagnostics deliberately carry the method, path, status and elapsed + // time only. The bearer token, the workspace header and request/response + // bodies are never logged — `-v` is a debugging aid, not a credential dump. + const method = init.method ?? 'GET'; + const startedAt = Date.now(); + log.verbose(`hosted → ${method} ${path}`); + try { + const response = await this.fetchImpl(`${this.apiBaseUrl}${path}`, { + ...init, + headers: { + accept: 'application/json', + authorization: `Bearer ${this.apiKey}`, + 'x-webcmd-session-protocol-version': String(HOSTED_SESSION_PROTOCOL_VERSION), + 'x-webcmd-client-capabilities': 'hosted-live-view-v1', + ...(init.body ? { 'content-type': 'application/json' } : {}), + ...(this.workspace ? { 'x-webcmd-workspace': this.workspace } : {}), + ...(init.headers ?? {}), + }, + }); + log.verbose(`hosted ← ${method} ${path} ${response.status} (${Date.now() - startedAt}ms)`); + return response; + } catch (error) { + log.verbose( + `hosted ✖ ${method} ${path} failed after ${Date.now() - startedAt}ms: ` + + `${error instanceof Error ? error.message : String(error)}`, + ); + throw error; + } } private async requestText(path: string): Promise { diff --git a/src/hosted/runner.test.ts b/src/hosted/runner.test.ts index adad48a2..c3096823 100644 --- a/src/hosted/runner.test.ts +++ b/src/hosted/runner.test.ts @@ -5,7 +5,7 @@ import path from 'node:path'; import { Writable, type WritableOptions } from 'node:stream'; import type { Command } from 'commander'; import yaml from 'js-yaml'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { browserCommandCatalog } from '../browser/command-catalog.js'; import { buildHostedContract } from './contract.js'; import { rejectPositionalBrowserSessionArgv } from '../cli-argv-preprocess.js'; @@ -1636,6 +1636,76 @@ describe('runHostedCli', () => { ].join('\n')); }); + // Hosted dispatch parsed -v and then dropped it, so the flag local mode honours + // was a silent no-op in hosted mode (#174). + describe('hosted verbose mode', () => { + let written: string[] = []; + + function runWhoami(argv: string[]) { + return runHostedCli(argv, { + config: makeHostedConfig({ apiBaseUrl: 'https://api.example.com', apiKey: 'hosted-secret-key' }), + stdout: sink().stream, + fetchImpl: async (url) => (String(url).endsWith('/v1/manifest') + ? manifestResponse() + : executionResponse({ result: [{ username: 'octocat' }], columns: ['username'] })), + }); + } + + beforeEach(() => { + delete process.env.WEBCMD_VERBOSE; + written = []; + vi.spyOn(process.stderr, 'write').mockImplementation((chunk: unknown) => { + written.push(String(chunk)); + return true; + }); + }); + + afterEach(() => { + delete process.env.WEBCMD_VERBOSE; + vi.restoreAllMocks(); + }); + + it('turns on verbose mode and emits request diagnostics for -v', async () => { + await expect(runWhoami(['github', 'whoami', '-f', 'json', '-v'])).resolves.toMatchObject({ exitCode: 0 }); + + expect(process.env.WEBCMD_VERBOSE).toBe('1'); + const output = written.join(''); + expect(output).toContain('hosted → POST /v1/execute'); + expect(output).toMatch(/hosted ← POST \/v1\/execute 200 \(\d+ms\)/); + expect(output).not.toContain('hosted-secret-key'); + }); + + it('emits no diagnostics without -v', async () => { + await expect(runWhoami(['github', 'whoami', '-f', 'json'])).resolves.toMatchObject({ exitCode: 0 }); + + expect(process.env.WEBCMD_VERBOSE).toBeUndefined(); + expect(written.join('')).not.toContain('hosted →'); + }); + + // The wire body is a server contract; -v is a local concern and must not + // start appearing as an unknown field in execute requests. + it('keeps verbose out of the /v1/execute request body', async () => { + const bodies: unknown[] = []; + await runHostedCli(['github', 'whoami', '-f', 'json', '-v'], { + config: makeHostedConfig({ apiBaseUrl: 'https://api.example.com', apiKey: 'key' }), + stdout: sink().stream, + fetchImpl: async (url, init) => { + if (init?.body) bodies.push(JSON.parse(String(init.body)) as unknown); + return String(url).endsWith('/v1/manifest') + ? manifestResponse() + : executionResponse({ result: [{ username: 'octocat' }], columns: ['username'] }); + }, + }); + + expect(bodies.at(-1)).toEqual({ + command: 'github/whoami', + args: {}, + format: 'json', + trace: 'off', + }); + }); + }); + it('uploads local file args, runs a prepared execution, and materializes hosted output artifacts', async () => { const tempDir = await mkdtemp(path.join(tmpdir(), 'webcmd-hosted-files-')); try { diff --git a/src/hosted/runner.ts b/src/hosted/runner.ts index 459dc321..d4adfd24 100644 --- a/src/hosted/runner.ts +++ b/src/hosted/runner.ts @@ -22,6 +22,7 @@ import { } from '../completion-shared.js'; import { CliError, ConfigError, EXIT_CODES, toEnvelope } from '../errors.js'; import { getRequestedHelpFormat, renderStructuredHelp } from '../help.js'; +import { enableVerbose } from '../logger.js'; import { findPackageRoot } from '../package-paths.js'; import { formatErrorEnvelope, render as renderOutput } from '../output.js'; import { StreamWriteError, writeToStream } from '../stream-write.js'; @@ -443,6 +444,11 @@ async function dispatchHosted( if (command.clientOwned) { throw new Error(`Internal invariant: client-owned command ${command.command} reached hosted dispatch.`); } + // Hosted dispatch parsed `-v` but never acted on it, so the flag that local + // mode honours was a silent no-op here (#174). Applying it before the request + // lights up the client's HTTP diagnostics on the same env contract local mode + // uses, keeping the two modes' verbose behaviour aligned. + enableVerbose(parsed.verbose); const startTime = now(); const response = command.browser || hasPresentFileArgument(command, parsed.args) diff --git a/src/logger.test.ts b/src/logger.test.ts index 86cb4eb7..ff7aa9bc 100644 --- a/src/logger.test.ts +++ b/src/logger.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { log } from './logger.js'; +import { enableVerbose, isVerbose, log } from './logger.js'; describe('log', () => { let stderrSpy: ReturnType; @@ -58,3 +58,40 @@ describe('log', () => { ]); }); }); + +describe('enableVerbose', () => { + beforeEach(() => { + delete process.env.WEBCMD_VERBOSE; + }); + + afterEach(() => { + delete process.env.WEBCMD_VERBOSE; + }); + + it('turns verbose mode on', () => { + expect(isVerbose()).toBe(false); + enableVerbose(); + expect(process.env.WEBCMD_VERBOSE).toBe('1'); + expect(isVerbose()).toBe(true); + }); + + it('turns verbose mode on when passed true', () => { + enableVerbose(true); + expect(isVerbose()).toBe(true); + }); + + it('leaves the environment untouched when passed false', () => { + enableVerbose(false); + expect(process.env.WEBCMD_VERBOSE).toBeUndefined(); + expect(isVerbose()).toBe(false); + }); + + // An explicit WEBCMD_VERBOSE=1 in the environment must survive a command that + // simply omits -v, otherwise exporting the variable for a whole shell session + // would be silently cancelled by every flagless invocation. + it('does not clear an environment-provided verbose mode', () => { + process.env.WEBCMD_VERBOSE = '1'; + enableVerbose(false); + expect(isVerbose()).toBe(true); + }); +}); diff --git a/src/logger.ts b/src/logger.ts index 4f7423cb..142f0a7d 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -10,6 +10,23 @@ export function isVerbose(): boolean { return value !== '' && value !== '0' && value !== 'false' && value !== 'no' && value !== 'off'; } +/** + * Turn on verbose diagnostics for the rest of the process. + * + * `-v` is parsed at several independent entry points — local Commander actions, + * hosted dispatch, raw browser leaves, auth probes — while every consumer reads + * the single `WEBCMD_VERBOSE` environment contract that `isVerbose()` defines. + * Routing each entry point through this helper keeps them from drifting, and + * lets child processes (adapter subprocesses, `browser verify`) inherit the mode. + * + * Passing `false` leaves the environment untouched rather than clearing it, so + * an explicit `WEBCMD_VERBOSE=1` in the environment still wins when the flag is + * simply absent from the command line. + */ +export function enableVerbose(enabled: boolean = true): void { + if (enabled) process.env.WEBCMD_VERBOSE = '1'; +} + export const log = { /** Informational message (always shown) */ info(msg: string): void {