diff --git a/devlog/_plan/260814_260814-strict-start-parser/000_plan.md b/devlog/_plan/260814_260814-strict-start-parser/000_plan.md new file mode 100644 index 0000000..9f7c785 --- /dev/null +++ b/devlog/_plan/260814_260814-strict-start-parser/000_plan.md @@ -0,0 +1,39 @@ +# 000 — strict-start-parser: Plan + +## Objective + +Fix the silent-ignore bug where agbrowse start --profile default passes without +error because the start command parseArgs uses strict: false. Users think they are +connecting to their real Chrome profile but agbrowse always uses its managed +profile at BROWSER_AGENT_HOME/browser-profile. + +Evidence: ChatGPT Pro analysis (conversation 6a7f0d58) plus codebase verification +at skills/browser/browser.mjs:2473-2493 confirming strict: false. + +## Loop-spec + +- Loop archetype: verifier-defined (pass/fail) +- Trigger: agbrowse start --profile default silently ignores --profile +- Goal: Unknown/unsupported flags on start produce a clear error +- Non-goals: Changing strict mode on OTHER commands; implementing connect command +- Verifier: npx vitest run test/integration/cli-lifecycle.test.mjs (exit 0) +- Stop condition: Tests pass, typecheck passes, manual CLI test confirms rejection +- Write scope: skills/browser/browser.mjs, test/integration/cli-lifecycle.test.mjs +- Out-of-scope: Other parseArgs sites (17 total all strict: false), README.md + +## Work-phase map + +Single PABCD cycle (C2): + +| WP | Doc | Slice | Depends on | +|----|-----|-------|------------| +| 1 | 010 | strict parseArgs for start plus tests | none | + +## Accept criteria + +1. agbrowse start --profile default exits non-zero with error mentioning profile +2. agbrowse start --headde exits non-zero (typo rejection) +3. agbrowse start --headed still works (no regression) +4. Regression tests added to cli-lifecycle.test.mjs +5. npx vitest run test/integration/cli-lifecycle.test.mjs passes +6. Typecheck passes diff --git a/devlog/_plan/260814_260814-strict-start-parser/010_phase1.md b/devlog/_plan/260814_260814-strict-start-parser/010_phase1.md new file mode 100644 index 0000000..2d182fe --- /dev/null +++ b/devlog/_plan/260814_260814-strict-start-parser/010_phase1.md @@ -0,0 +1,61 @@ +# 010 — Strict parseArgs for start command + +## MODIFY: skills/browser/browser.mjs (lines 2473-2493) + +Change the start case to use strict: true and wrap in try/catch for friendly errors. + +Before (line 2474-2484): + case 'start': { + const { values } = parseArgs({ + args: process.argv.slice(3), + options: { ... }, + strict: false, + }); + +After: + case 'start': { + let values; + try { + ({ values } = parseArgs({ + args: process.argv.slice(3), + options: { ... }, + strict: true, + allowPositionals: false, + })); + } catch (e) { + const msg = e.message || String(e); + if (/profile/i.test(msg)) { + console.error( + 'agbrowse start --profile is not supported.\n' + + 'agbrowse uses its dedicated persistent profile at\n' + + ' BROWSER_AGENT_HOME/browser-profile\n' + + ' (default: ~/.browser-agent/browser-profile)\n' + + 'Use BROWSER_AGENT_HOME and CDP_PORT for separate profiles.' + ); + } else { + console.error('agbrowse start: ' + msg); + } + process.exit(1); + } + +## MODIFY: test/integration/cli-lifecycle.test.mjs + +Add after existing stop test: + + it('rejects --profile flag with a clear error', async () => { + const r = await execBrowser(['start', '--profile', 'default'], { env }); + expect(r.code).not.toBe(0); + expect(r.stderr).toContain('--profile is not supported'); + }); + + it('rejects unknown flags (typo protection)', async () => { + const r = await execBrowser(['start', '--headde'], { env }); + expect(r.code).not.toBe(0); + }); + +## Verification (C) + +1. npx vitest run test/integration/cli-lifecycle.test.mjs (exit 0) +2. node skills/browser/browser.mjs start --profile default (should exit 1, stderr contains 'not supported') +3. node skills/browser/browser.mjs start --headde (should exit 1) + diff --git a/package.json b/package.json index c5b418d..f3473d0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,7 @@ { "name": "agbrowse", - "version": "0.1.23", + "version": "0.1.23", + "version": "0.1.24", "description": "Standalone Chrome/CDP browser automation and web-ai workflow skills for AI agents.", "license": "MIT", "private": false, diff --git a/skills/browser/browser.mjs b/skills/browser/browser.mjs index b6d4784..9406c00 100755 --- a/skills/browser/browser.mjs +++ b/skills/browser/browser.mjs @@ -2471,17 +2471,36 @@ try { break; } case 'start': { - const { values } = parseArgs({ - args: process.argv.slice(3), - options: { - port: { type: 'string', default: String(DEFAULT_CDP_PORT) }, - headless: { type: 'boolean', default: false }, - headed: { type: 'boolean', default: false }, - 'chrome-path': { type: 'string' }, - 'heavy-site-compat': { type: 'boolean', default: false }, - 'keep-bg-networking': { type: 'boolean', default: false }, - }, strict: false, - }); + let values; + try { + ({ values } = parseArgs({ + args: process.argv.slice(3), + options: { + port: { type: 'string', default: String(DEFAULT_CDP_PORT) }, + headless: { type: 'boolean', default: false }, + headed: { type: 'boolean', default: false }, + 'chrome-path': { type: 'string' }, + 'heavy-site-compat': { type: 'boolean', default: false }, + 'keep-bg-networking': { type: 'boolean', default: false }, + }, + strict: true, + allowPositionals: false, + })); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + if (/profile/i.test(msg)) { + console.error( + '\`agbrowse start --profile\` is not supported.\n' + + 'agbrowse uses its dedicated persistent profile at\n' + + ' $BROWSER_AGENT_HOME/browser-profile\n' + + ' (default: ~/.browser-agent/browser-profile)\n' + + 'Use BROWSER_AGENT_HOME and CDP_PORT for another managed profile.' + ); + } else { + console.error('agbrowse start: ' + msg); + } + process.exit(1); + } if (values['heavy-site-compat']) process.env.AGBROWSE_HEAVY_SITE_COMPAT = '1'; if (values['keep-bg-networking']) process.env.AGBROWSE_KEEP_BG_NETWORKING = '1'; await launchChrome(Number(values.port), { diff --git a/test/integration/cli-lifecycle.test.mjs b/test/integration/cli-lifecycle.test.mjs index a22bdc0..10d6e1d 100644 --- a/test/integration/cli-lifecycle.test.mjs +++ b/test/integration/cli-lifecycle.test.mjs @@ -57,4 +57,22 @@ describe.sequential('browser lifecycle regressions', () => { const status = await execBrowser(['status'], { env }); expect(status.stdout).toContain('running: false'); }); + + it('rejects --profile flag with a clear error (strict parseArgs)', async () => { + const r = await execBrowser(['start', '--profile', 'default'], { env }); + expect(r.code).not.toBe(0); + expect(r.stderr).toContain('--profile'); + expect(r.stderr).toContain('not supported'); + }); + + it('rejects unknown flags like --headde (typo protection)', async () => { + const r = await execBrowser(['start', '--headde'], { env }); + expect(r.code).not.toBe(0); + expect(r.stderr).toContain('Unknown option'); + }); + + it('rejects positional arguments on start', async () => { + const r = await execBrowser(['start', 'default'], { env }); + expect(r.code).not.toBe(0); + }); });