From ffc16665e5dc926d0ba41960f437e1f075306e0e Mon Sep 17 00:00:00 2001 From: javis-bot Date: Wed, 24 Jun 2026 19:17:46 +0900 Subject: [PATCH] fix(controlBrowser): never report moveMouse/search success without a real move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit moveMouse returned ok:true even when humanHover did nothing (no on-screen box) or the selector never matched — recreating the "claims it moved but didn't" bug. Now: humanHover returns a boolean (and brings the element into view first); moveMouse returns ok:false when the target isn't found or has no on-screen box, and when site=naver/... whose box isn't on the current page it navigates to the site home first before hovering. search now reports input=human|api-fallback|api so a silent fallback to cursor-less DOM input is visible, and the tool surfaces that note in the reply instead of implying a human-like search happened. --- bot/scripts/stream-test/chrome-control.mjs | 37 ++++++++++++++++++--- bot/scripts/stream-test/human.mjs | 10 ++++-- src/jarvis/tools/builtin/control_browser.py | 7 +++- tests/test_control_browser.py | 19 +++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) diff --git a/bot/scripts/stream-test/chrome-control.mjs b/bot/scripts/stream-test/chrome-control.mjs index d1e8558..755ddc2 100644 --- a/bot/scripts/stream-test/chrome-control.mjs +++ b/bot/scripts/stream-test/chrome-control.mjs @@ -125,23 +125,31 @@ try { // 2) Click the on-page search box, type the query, submit. const box = page.locator(s.box).first(); await box.waitFor({ state: 'visible', timeout: 15000 }).catch(() => {}); + // Report which input path actually ran: 'human' = real xdotool cursor + // move + char typing; 'api-fallback' = the humanClick path threw and we + // fell back to cursor-less DOM click/fill; 'api' = no xdotool at all. This + // makes "did the cursor really move" verifiable from the result. + let searchInput; if (HAS_XDOTOOL && cmd.human !== false) { try { await human.humanClick(page, box); await human.humanType(q); await human.pressKey('Return'); + searchInput = 'human'; } catch { + searchInput = 'api-fallback'; await box.click().catch(() => {}); await box.fill(q).catch(() => {}); await page.keyboard.press('Enter').catch(() => {}); } } else { + searchInput = 'api'; await box.click().catch(() => {}); await box.fill(q); await page.keyboard.press('Enter'); } await page.waitForLoadState('domcontentloaded').catch(() => {}); - out({ ok: true, site: SITES[siteKey] ? siteKey : 'google', query: q, url: page.url(), title: await page.title().catch(() => '') }); + out({ ok: true, site: SITES[siteKey] ? siteKey : 'google', query: q, url: page.url(), title: await page.title().catch(() => ''), input: searchInput }); break; } @@ -212,7 +220,10 @@ try { // Move/hover the REAL cursor onto an element WITHOUT clicking. Target is a // CSS selector, or site=naver/google/... for that site's search box. // Only meaningful with xdotool (the visible cursor); with no xdotool there - // is no cursor to move, so report that rather than faking success. + // is no cursor to move, so report that rather than faking success. Every + // failure to actually move (no xdotool, selector never matches, element + // has no on-screen box) returns ok:false — we must never claim the cursor + // moved when it did not (the exact bug the user reported). const siteKey = String(cmd.site || '').toLowerCase(); const selector = String(cmd.selector || '').trim() || (SITES[siteKey] ? SITES[siteKey].box : ''); if (!selector) throw new Error('moveMouse: no selector or known site'); @@ -221,9 +232,25 @@ try { break; } await front(page); - const locator = page.locator(selector).first(); - await locator.waitFor({ state: 'visible', timeout: 10000 }).catch(() => {}); - await human.humanHover(page, locator); + let locator = page.locator(selector).first(); + let visible = await locator.waitFor({ state: 'visible', timeout: 8000 }).then(() => true).catch(() => false); + // A named site whose search box isn't on the current page: go to its home + // first (real omnibox), then target the box there. + if (!visible && SITES[siteKey]) { + try { await human.navigateOmnibox(SITES[siteKey].home); await page.waitForLoadState('domcontentloaded').catch(() => {}); } + catch { await page.goto(SITES[siteKey].home, { waitUntil: 'domcontentloaded' }).catch(() => {}); } + locator = page.locator(SITES[siteKey].box).first(); + visible = await locator.waitFor({ state: 'visible', timeout: 8000 }).then(() => true).catch(() => false); + } + if (!visible) { + out({ ok: false, error: `moveMouse: target not found (${cmd.selector || siteKey})` }); + break; + } + const moved = await human.humanHover(page, locator); + if (!moved) { + out({ ok: false, error: 'moveMouse: element has no on-screen box; cursor not moved' }); + break; + } out({ ok: true, target: cmd.selector || siteKey, input: 'human' }); break; } diff --git a/bot/scripts/stream-test/human.mjs b/bot/scripts/stream-test/human.mjs index 9cc40bd..ea3cc9c 100644 --- a/bot/scripts/stream-test/human.mjs +++ b/bot/scripts/stream-test/human.mjs @@ -136,14 +136,18 @@ export async function navigateOmnibox(text) { } // Move the real cursor over an element (hover, no click) - e.g. to reveal a -// video player's controls or to focus it for a keyboard shortcut. +// video player's controls or to focus it for a keyboard shortcut. Returns true +// only if the element had an on-screen box and the cursor was actually moved; +// returns false when there is nothing to move to (so callers must not report +// success). Brings the element into view with a real wheel scroll first. export async function humanHover(page, locator) { - const box = await locator.boundingBox().catch(() => null); - if (!box) return; + const box = await bringIntoView(page, locator); + if (!box) return false; const g = await page.evaluate(() => ({ sx: window.screenX, sy: window.screenY, ow: window.outerWidth, oh: window.outerHeight, iw: window.innerWidth, ih: window.innerHeight })); const bx = Math.max(0, Math.round((g.ow - g.iw) / 2)); const oy = g.sy + Math.max(0, g.oh - g.ih - bx); await humanMove(Math.round(g.sx + bx + box.x + box.width * 0.5), Math.round(oy + box.y + box.height * 0.4)); + return true; } export { sleep, rand }; diff --git a/src/jarvis/tools/builtin/control_browser.py b/src/jarvis/tools/builtin/control_browser.py index 56a1041..63ce2e5 100644 --- a/src/jarvis/tools/builtin/control_browser.py +++ b/src/jarvis/tools/builtin/control_browser.py @@ -156,7 +156,12 @@ class ControlBrowserTool(Tool): if action == "navigate": return f"브라우저에서 {data.get('url', args.get('url'))} 로 이동했습니다." if action == "search": - return f"{data.get('site', '')}에서 '{data.get('query', args.get('query'))}'를 검색해 화면에 띄웠습니다." + base = f"{data.get('site', '')}에서 '{data.get('query', args.get('query'))}'를 검색해 화면에 띄웠습니다." + # Flag when the real cursor path didn't run, so a silent fallback to + # cursor-less DOM input is visible rather than reported as "human". + if data.get("input") in ("api", "api-fallback"): + base += " (참고: 실제 마우스 커서 이동 없이 처리됨)" + return base if action in ("back", "forward", "refresh"): return f"브라우저: {action} 완료 ({data.get('url', '')})." if action in ("status", "listTabs"): diff --git a/tests/test_control_browser.py b/tests/test_control_browser.py index f84c718..5badaeb 100644 --- a/tests/test_control_browser.py +++ b/tests/test_control_browser.py @@ -35,3 +35,22 @@ def test_description_distinguishes_cursor_paths(tool): desc = tool.description assert "moveMouse" in desc assert "address bar" in desc # navigate is described as address-bar typing + + +def test_search_summary_flags_cursorless_fallback(tool): + # When the real xdotool cursor path didn't run, the summary must say so + # rather than implying a human-like search happened. + human = tool._summarise("search", {"query": "날씨"}, {"ok": True, "site": "naver", "query": "날씨", "input": "human"}) + assert "참고: 실제 마우스" not in human + + fell_back = tool._summarise("search", {"query": "날씨"}, {"ok": True, "site": "naver", "query": "날씨", "input": "api-fallback"}) + assert "실제 마우스 커서 이동 없이" in fell_back + + +def test_movemouse_summary_only_runs_on_success(tool): + # _summarise is only called on ok:true; an ok:false (target not found / no + # xdotool) is handled by run() as a failure reply, so a failed move can no + # longer be reported as "moved". Sanity-check the success summary names a + # target rather than a placeholder when one is present. + summary = tool._summarise("moveMouse", {"selector": "#query"}, {"ok": True, "target": "#query"}) + assert "#query" in summary