Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions devlog/_plan/260814_260814-strict-start-parser/000_plan.md
Original file line number Diff line number Diff line change
@@ -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
61 changes: 61 additions & 0 deletions devlog/_plan/260814_260814-strict-start-parser/010_phase1.md
Original file line number Diff line number Diff line change
@@ -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)

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
41 changes: 30 additions & 11 deletions skills/browser/browser.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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), {
Expand Down
18 changes: 18 additions & 0 deletions test/integration/cli-lifecycle.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Loading