fix: support URL parameter in tab new command (#64)
* fix: support URL parameter in tab new command The CLI was correctly sending the URL parameter when running `agent-browser tab new <url>`, but the TypeScript daemon was ignoring it because: 1. The schema didn't include the url field (stripped during validation) 2. The TabNewCommand type didn't have a url property 3. The handler didn't pass the URL to browser.newTab() 4. browser.newTab() didn't accept or use a URL parameter This fix adds URL support throughout the chain so that `agent-browser tab new https://example.com` now correctly opens a new tab and navigates to the specified URL. Fixes #62 * fix: omit url field when not provided in tab new command Previously, the CLI always sent "url": null when no URL was provided, which caused Zod validation to fail with "Expected string, received null". Now the url field is only included when a URL is actually provided. Fixes issue reported by @ctate in PR review. * refactor: move navigation logic from BrowserManager to handleTabNew Address review feedback: - Add .min(1) to URL validation for consistency with navigateSchema - Keep BrowserManager.newTab() simple (single responsibility) - Handle navigation in handleTabNew following same pattern as handleNavigate
This commit is contained in:
+15
-1
@@ -367,7 +367,13 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
|
|||||||
// === Tabs ===
|
// === Tabs ===
|
||||||
"tab" => {
|
"tab" => {
|
||||||
match rest.get(0).map(|s| *s) {
|
match rest.get(0).map(|s| *s) {
|
||||||
Some("new") => Ok(json!({ "id": id, "action": "tab_new", "url": rest.get(1) })),
|
Some("new") => {
|
||||||
|
let mut cmd = json!({ "id": id, "action": "tab_new" });
|
||||||
|
if let Some(url) = rest.get(1) {
|
||||||
|
cmd["url"] = json!(url);
|
||||||
|
}
|
||||||
|
Ok(cmd)
|
||||||
|
}
|
||||||
Some("list") => Ok(json!({ "id": id, "action": "tab_list" })),
|
Some("list") => Ok(json!({ "id": id, "action": "tab_list" })),
|
||||||
Some("close") => {
|
Some("close") => {
|
||||||
Ok(json!({ "id": id, "action": "tab_close", "index": rest.get(1).and_then(|s| s.parse::<i32>().ok()) }))
|
Ok(json!({ "id": id, "action": "tab_close", "index": rest.get(1).and_then(|s| s.parse::<i32>().ok()) }))
|
||||||
@@ -1238,6 +1244,14 @@ mod tests {
|
|||||||
fn test_tab_new() {
|
fn test_tab_new() {
|
||||||
let cmd = parse_command(&args("tab new"), &default_flags()).unwrap();
|
let cmd = parse_command(&args("tab new"), &default_flags()).unwrap();
|
||||||
assert_eq!(cmd["action"], "tab_new");
|
assert_eq!(cmd["action"], "tab_new");
|
||||||
|
assert!(cmd.get("url").is_none(), "url should not be present when not provided");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_tab_new_with_url() {
|
||||||
|
let cmd = parse_command(&args("tab new https://example.com"), &default_flags()).unwrap();
|
||||||
|
assert_eq!(cmd["action"], "tab_new");
|
||||||
|
assert_eq!(cmd["url"], "https://example.com");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
+9
-1
@@ -26,6 +26,7 @@ import type {
|
|||||||
SelectCommand,
|
SelectCommand,
|
||||||
HoverCommand,
|
HoverCommand,
|
||||||
ContentCommand,
|
ContentCommand,
|
||||||
|
TabNewCommand,
|
||||||
TabSwitchCommand,
|
TabSwitchCommand,
|
||||||
TabCloseCommand,
|
TabCloseCommand,
|
||||||
WindowNewCommand,
|
WindowNewCommand,
|
||||||
@@ -709,10 +710,17 @@ async function handleClose(
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function handleTabNew(
|
async function handleTabNew(
|
||||||
command: Command & { action: 'tab_new' },
|
command: TabNewCommand,
|
||||||
browser: BrowserManager
|
browser: BrowserManager
|
||||||
): Promise<Response<TabNewData>> {
|
): Promise<Response<TabNewData>> {
|
||||||
const result = await browser.newTab();
|
const result = await browser.newTab();
|
||||||
|
|
||||||
|
// Navigate to URL if provided (same pattern as handleNavigate)
|
||||||
|
if (command.url) {
|
||||||
|
const page = browser.getPage();
|
||||||
|
await page.goto(command.url, { waitUntil: 'domcontentloaded' });
|
||||||
|
}
|
||||||
|
|
||||||
return successResponse(command.id, result);
|
return successResponse(command.id, result);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -373,6 +373,14 @@ describe('parseCommand', () => {
|
|||||||
expect(result.success).toBe(true);
|
expect(result.success).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should parse tab_new with url', () => {
|
||||||
|
const result = parseCommand(cmd({ id: '1', action: 'tab_new', url: 'https://example.com' }));
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
if (result.success) {
|
||||||
|
expect((result.command as { url?: string }).url).toBe('https://example.com');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it('should parse tab_list', () => {
|
it('should parse tab_list', () => {
|
||||||
const result = parseCommand(cmd({ id: '1', action: 'tab_list' }));
|
const result = parseCommand(cmd({ id: '1', action: 'tab_list' }));
|
||||||
expect(result.success).toBe(true);
|
expect(result.success).toBe(true);
|
||||||
|
|||||||
@@ -730,6 +730,7 @@ const closeSchema = baseCommandSchema.extend({
|
|||||||
// Tab/Window schemas
|
// Tab/Window schemas
|
||||||
const tabNewSchema = baseCommandSchema.extend({
|
const tabNewSchema = baseCommandSchema.extend({
|
||||||
action: z.literal('tab_new'),
|
action: z.literal('tab_new'),
|
||||||
|
url: z.string().min(1).optional(),
|
||||||
});
|
});
|
||||||
|
|
||||||
const tabListSchema = baseCommandSchema.extend({
|
const tabListSchema = baseCommandSchema.extend({
|
||||||
|
|||||||
@@ -772,6 +772,7 @@ export interface CloseCommand extends BaseCommand {
|
|||||||
// Tab/Window commands
|
// Tab/Window commands
|
||||||
export interface TabNewCommand extends BaseCommand {
|
export interface TabNewCommand extends BaseCommand {
|
||||||
action: 'tab_new';
|
action: 'tab_new';
|
||||||
|
url?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface TabListCommand extends BaseCommand {
|
export interface TabListCommand extends BaseCommand {
|
||||||
|
|||||||
Reference in New Issue
Block a user