diff --git a/README.md b/README.md index c2fb529..08889e3 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,7 @@ FLASH (**F**ast **L**ocal **A**gent **SH**ell) CLI is an AI-powered command-line - **`flash://` Links**: Open Flash from a browser or another app with a prompt ready to go (`flash://?prompt=What+is+Python`). - **Image Recognition**: Send a local image to a vision-capable model with `/image [prompt]`, or let the AI open one itself with its `view_image` tool. - **Page Screenshots**: The AI renders a page it built in a headless browser with its `screenshot` tool and looks at the result, so it can see a broken layout instead of guessing from the HTML. +- **Page Control**: The AI opens a page with `open_page` and then clicks buttons, fills forms, presses keys, and runs JavaScript on it with `interact`, seeing a fresh screenshot, the page's elements, and its console errors after every step, so it can debug what a page *does*, not just how it looks. - **Context Management**: Automatic history trimming to stay within token limits. - **Markdown Support**: Rich formatting for AI responses in the terminal. @@ -135,7 +136,7 @@ python run.py - `/clear`: Clear the conversation history. - `/image [prompt]`: Send a local image to the model. - `/version`: Show the current version and check GitHub for updates. -- `/update`: Update Flash to the latest version (pipx installs only). +- `/update`: Update Flash to the latest version (requires pipx). - `/bye`: Exit the application. ### Image Recognition @@ -181,7 +182,29 @@ page with `full_page`, and reports any JavaScript errors the page threw while rendering, which is usually what explains a section that came out empty. -Screenshots need Playwright's Chromium, which `install.sh` and +### Clicking through a page + +A screenshot is a still picture, so for a page with buttons or a form the +AI opens it with `open_page` and then drives it with `interact`, one +action per call: + +``` +Open ~/Desktop/signup.html, fill in the form, submit it, and tell me why +the confirmation never shows up. +``` + +The browser stays open between calls, so the page keeps its state while +the AI works through a flow. `interact` takes an `action` (`click`, +`fill`, `press`, `hover`, `select`, `scroll`, `wait`, `eval`, `back`, +`reload`, `close`) and a `selector`, which can be the number Flash prints +beside each element, a CSS selector, or the text on the element itself. +Every call answers with where the page is now, what can be clicked or +typed into next, and the JavaScript errors the page threw, with a +screenshot attached. The `eval` action runs JavaScript against the live +page and returns the result, which is how the AI inspects state a picture +cannot show. + +Both tools need Playwright's Chromium, which `install.sh` and `install.ps1` download for you. Installing Flash another way means running it yourself: @@ -194,9 +217,8 @@ playwright install chromium Flash checks `main` on GitHub for a newer version on startup and shows it in the banner if one is available. Run `/version` anytime to check on demand, or `/update` to install it. Flash re-runs the same pipx-based -steps `install.sh` uses, so it only works for installs done via the -quick-install script. If you cloned the repo manually, update with -`git pull` instead. +steps `install.sh` uses, so it needs pipx on PATH. If you cloned the repo +manually, update with `git pull` instead. You can also check and update from outside the REPL: diff --git a/flash/ai.py b/flash/ai.py index 1c972a4..3034699 100644 --- a/flash/ai.py +++ b/flash/ai.py @@ -254,7 +254,10 @@ def _direct_shell_command( # read already caps its own output by whole lines and tells the model how # to page on; the middle-out trim below would silently gut a file read. -_SELF_LIMITING_TOOLS = {"read"} +# The page tools cap themselves too, and their element list is only useful +# whole: a trim through the middle of it takes away the very numbers the +# next click has to name. +_SELF_LIMITING_TOOLS = {"read", "open_page", "interact"} def _trim_tool_output(text: str, name: str = "") -> str: diff --git a/flash/browser.py b/flash/browser.py index 7cfc259..00f6ae7 100644 --- a/flash/browser.py +++ b/flash/browser.py @@ -1,14 +1,22 @@ -"""Headless browser screenshots for Flash CLI. +"""Headless browser screenshots and page control for Flash CLI. Playwright drives a real Chromium so the model can look at a page it built instead of guessing from the source. The import is deferred to the -moment a screenshot is asked for, because Playwright is slow to import -and Flash starts fine without it; a missing install turns into a tool -result the model can read and relay rather than a crash at startup. +moment a browser is asked for, because Playwright is slow to import and +Flash starts fine without it; a missing install turns into a tool result +the model can read and relay rather than a crash at startup. + +`capture` is the one-shot photograph. The session half of this module is +for the pages a picture cannot answer questions about: it keeps one +Chromium open across tool calls so the model can click a button, fill a +field, run JavaScript against the live DOM, and look again at what its +last action actually did. """ +import atexit +import json from pathlib import Path -from typing import Union +from typing import Any, Union from urllib.parse import urlparse PAGE_EXTENSIONS = {".html", ".htm", ".xhtml", ".svg"} @@ -85,7 +93,11 @@ def on_console(message) -> None: page.on("pageerror", lambda exc: problems.append(f"page error: {exc}")) -def _settle(page, wait_ms: int) -> None: +def _settle( + page, + wait_ms: int, + idle_timeout: int = NAVIGATION_TIMEOUT_MS, +) -> None: """Give fonts, layout, and any intro animation time to finish.""" # A page that keeps a socket open or ships no web fonts is still @@ -93,7 +105,7 @@ def _settle(page, wait_ms: int) -> None: try: page.wait_for_load_state( "networkidle", - timeout=NAVIGATION_TIMEOUT_MS, + timeout=idle_timeout, ) except Exception: # noqa: BLE001, S110 pass @@ -175,3 +187,492 @@ def _launch_reason(exc: Exception) -> str: return BROWSER_HINT return f"Chromium would not start: {_first_line(exc)}" + + +# A click on a button that never appears should fail fast; only a whole +# navigation is worth waiting the longer time for. +ACTION_TIMEOUT_MS = 5000 + +# A page can carry hundreds of links. Enough of them to work with beats a +# list the model has to wade through. +MAX_ELEMENTS = 40 + +NO_PAGE = "No page is open. Open one with the open_page tool first." + +# Numbering the interactive elements and stamping the number onto each +# one gives the model a selector it cannot get wrong. The stamp is a data +# attribute, so it changes nothing about how the page looks or behaves, +# and it is rewritten on every scan because the DOM moves under it. +_SCAN_JS = """ +(limit) => { + const wanted = [ + 'a[href]', 'button', 'input', 'select', 'textarea', 'summary', + '[role="button"]', '[role="link"]', '[role="tab"]', '[role="checkbox"]', + '[onclick]', '[contenteditable]', + ].join(', '); + const all = document.querySelectorAll(wanted); + const items = []; + let id = 0; + for (const el of all) { + const box = el.getBoundingClientRect(); + const style = window.getComputedStyle(el); + if (!box.width || !box.height) continue; + if (style.visibility === 'hidden' || style.display === 'none') continue; + id += 1; + el.setAttribute('data-flash-id', String(id)); + const label = el.getAttribute('aria-label') + || (el.innerText || '').trim() + || el.value + || el.getAttribute('placeholder') + || el.getAttribute('title') + || el.getAttribute('name') + || ''; + items.push({ + id: id, + tag: el.tagName.toLowerCase(), + type: el.getAttribute('type') || '', + label: String(label).replace(/\\s+/g, ' ').trim().slice(0, 60), + disabled: el.disabled === true, + }); + if (items.length >= limit) break; + } + return {items: items, total: all.length}; +} +""" + + +class PageProblem(Exception): + """Something the model asked of the page that the page would not do.""" + + +class _Session: + """A Chromium that outlives the tool call which opened it.""" + + def __init__(self, driver, browser) -> None: + self.driver = driver + self.browser = browser + self.page: Any = None + self.problems: list[str] = [] + + def drain(self) -> list[str]: + """Hand over the errors seen since the last time we asked.""" + + seen = list(self.problems) + self.problems.clear() + + return seen + + def shut(self) -> None: + # Shutting down is best-effort: a browser that has already + # crashed must not stop Flash from opening the next one. + for stop in (self.browser.close, self.driver.stop): + try: + stop() + except Exception: # noqa: BLE001, S110 + pass + + +_session: Union[_Session, None] = None # noqa: UP007, RUF100 + + +def is_open() -> bool: + """True while there is a live page to act on.""" + + if _session is None or _session.page is None: + return False + + try: + return not _session.page.is_closed() + except Exception: # noqa: BLE001 + return False + + +def open_page(url: str, *, width: int, height: int, wait_ms: int) -> str: + """Open `url` in a browser that stays open for later interaction. + + Returns `""` once the page has loaded, or the reason it has not. + """ + + global _session + + try: + from playwright.sync_api import Error as PlaywrightError + from playwright.sync_api import sync_playwright + except ImportError: + return PACKAGE_HINT + + # One page at a time: a second Chromium the model has forgotten about + # is a leak, not a feature. + close_session() + + try: + driver = sync_playwright().start() + except Exception as exc: # noqa: BLE001 + return f"Chromium would not start: {_first_line(exc)}" + + try: + browser = driver.chromium.launch() + except PlaywrightError as exc: + driver.stop() + return _launch_reason(exc) + + session = _Session(driver, browser) + + try: + page = browser.new_page(viewport={"width": width, "height": height}) + page.set_default_timeout(ACTION_TIMEOUT_MS) + session.page = page + _watch(page, session.problems) + page.goto(url, wait_until="load", timeout=NAVIGATION_TIMEOUT_MS) + _settle(page, wait_ms) + except PlaywrightError as exc: + session.shut() + return _first_line(exc) + + _session = session + + return "" + + +def interact( + action: str, + *, + selector: str = "", + value: str = "", + wait_ms: int = 0, +) -> tuple[str, str]: + """Do one thing to the open page. + + Returns `(note, "")` describing what happened, or `("", reason)` when + nothing did. + """ + + if not is_open(): + return "", NO_PAGE + + handler = _ACTIONS.get(action.strip().lower()) + + if handler is None: + return "", ( + f"Unknown action '{action}'. Use one of: " + + ", ".join(ACTIONS) + + "." + ) + + from playwright.sync_api import Error as PlaywrightError + + page = _session.page + + try: + note = handler(page, selector.strip(), value) + # The action may have navigated or started a fetch, so let the + # page catch up before anyone photographs it. + _settle(page, wait_ms, ACTION_TIMEOUT_MS) + except (PageProblem, PlaywrightError) as exc: + return "", _first_line(exc) + + return note, "" + + +def snapshot(out: Path, *, full_page: bool) -> str: + """Photograph the open page as it stands. Returns "" or the reason.""" + + if not is_open(): + return NO_PAGE + + from playwright.sync_api import Error as PlaywrightError + + try: + _session.page.screenshot(path=str(out), full_page=full_page) + except PlaywrightError as exc: + return _first_line(exc) + + if not out.is_file() or out.stat().st_size == 0: + return "Chromium wrote no screenshot of the open page." + + return "" + + +def elements() -> tuple[list[str], str]: + """Number what can be clicked or typed into. Returns `(lines, why)`.""" + + if not is_open(): + return [], NO_PAGE + + from playwright.sync_api import Error as PlaywrightError + + try: + found = _session.page.evaluate(_SCAN_JS, MAX_ELEMENTS) + except PlaywrightError as exc: + return [], _first_line(exc) + + items = found.get("items", []) + lines = [_describe(item) for item in items] + + hidden = found.get("total", 0) - len(items) + if hidden > 0: + lines.append(f"... and {hidden} more not listed") + + return lines, "" + + +def where() -> tuple[str, str]: + """The open page's `(url, title)`, or `("", "")` when none is open.""" + + if not is_open(): + return "", "" + + page = _session.page + + try: + return page.url, page.title() + except Exception: # noqa: BLE001 + return getattr(page, "url", ""), "" + + +def drain_problems() -> list[str]: + """The errors the page has thrown since the last time it was asked.""" + + return [] if _session is None else _session.drain() + + +def close_session() -> bool: + """Shut the open browser. True when there was one to shut.""" + + global _session + + session = _session + _session = None + + if session is None: + return False + + session.shut() + + return True + + +# Chromium runs in its own process, so leaving one behind would outlive +# Flash itself. +atexit.register(close_session) + + +def _describe(item: dict) -> str: + """One element as a line the model can read and then act on.""" + + kind = item.get("tag", "?") + if item.get("type"): + kind += ":" + item["type"] + + line = f"[{item.get('id')}] {kind}" + + if item.get("label"): + line += ' "' + item["label"] + '"' + + if item.get("disabled"): + line += " (disabled)" + + return line + + +def _locate(page, selector: str): + """Turn what the model typed into a locator for one element. + + A bare number is an id from the last element list. Anything else is + tried as a CSS selector first and as the visible text second, because + a model reaches for 'Sign in' far more readily than for + 'button.primary:nth-of-type(2)'. + """ + + if not selector: + raise PageProblem( + "That action needs a selector: an element number from the " + "list, a CSS selector, or the text on the element." + ) + + if selector.isdigit(): + stamped = page.locator('[data-flash-id="' + selector + '"]') + if not stamped.count(): + raise PageProblem( + f"There is no element {selector} on this page. The numbers " + "are handed out again after every action, so use the list " + "that came with the most recent result." + ) + + return stamped.first + + try: + css = page.locator(selector) + if css.count(): + return css.first + except Exception: # noqa: BLE001, S110 + pass # Not a selector Playwright understands; try it as text. + + by_text = page.get_by_text(selector) + if by_text.count(): + return by_text.first + + quoted = json.dumps(selector) + labelled = page.locator( + ", ".join( + "[" + name + "=" + quoted + "]" + for name in ("aria-label", "placeholder", "title", "name", "value") + ) + ) + if labelled.count(): + return labelled.first + + raise PageProblem( + f"Nothing on the page matches '{selector}'. Check the element list " + "in the last result and act on one of its numbers." + ) + + +def _act_click(page, selector: str, value: str) -> str: + _locate(page, selector).click(timeout=ACTION_TIMEOUT_MS) + + return f"Clicked {selector}." + + +def _act_fill(page, selector: str, value: str) -> str: + if not value: + raise PageProblem("fill needs the text to type, in value.") + + _locate(page, selector).fill(value, timeout=ACTION_TIMEOUT_MS) + + return f"Typed '{value}' into {selector}." + + +def _act_press(page, selector: str, value: str) -> str: + key = value.strip() or "Enter" + + if selector: + _locate(page, selector).press(key, timeout=ACTION_TIMEOUT_MS) + + return f"Pressed {key} on {selector}." + + page.keyboard.press(key) + + return f"Pressed {key}." + + +def _act_hover(page, selector: str, value: str) -> str: + _locate(page, selector).hover(timeout=ACTION_TIMEOUT_MS) + + return f"Hovered over {selector}." + + +def _act_select(page, selector: str, value: str) -> str: + if not value: + raise PageProblem("select needs the option to choose, in value.") + + _locate(page, selector).select_option(value, timeout=ACTION_TIMEOUT_MS) + + return f"Selected '{value}' in {selector}." + + +def _act_scroll(page, selector: str, value: str) -> str: + if selector: + _locate(page, selector).scroll_into_view_if_needed( + timeout=ACTION_TIMEOUT_MS, + ) + + return f"Scrolled {selector} into view." + + where_to = value.strip().lower() or "bottom" + + if where_to in {"bottom", "end", "down"}: + page.evaluate("window.scrollTo(0, document.body.scrollHeight)") + + return "Scrolled to the bottom of the page." + + if where_to in {"top", "start", "up"}: + page.evaluate("window.scrollTo(0, 0)") + + return "Scrolled to the top of the page." + + try: + pixels = int(float(where_to)) + except ValueError: + raise PageProblem( + "scroll takes 'top', 'bottom', or a number of pixels in value." + ) from None + + page.evaluate("(y) => window.scrollBy(0, y)", pixels) + + return f"Scrolled {pixels} pixels down the page." + + +def _act_wait(page, selector: str, value: str) -> str: + if selector: + _locate(page, selector).wait_for( + state="visible", + timeout=NAVIGATION_TIMEOUT_MS, + ) + + return f"{selector} is now visible." + + try: + pause = int(float(value.strip() or 1000)) + except ValueError: + pause = 1000 + + pause = min(pause, NAVIGATION_TIMEOUT_MS) + page.wait_for_timeout(pause) + + return f"Waited {pause} ms." + + +def _act_eval(page, selector: str, value: str) -> str: + if not value.strip(): + raise PageProblem("eval needs the JavaScript to run, in value.") + + return f"JavaScript returned: {_short(page.evaluate(value))}" + + +def _act_back(page, selector: str, value: str) -> str: + if page.go_back(wait_until="load", timeout=NAVIGATION_TIMEOUT_MS) is None: + return "There was nothing to go back to." + + return f"Went back to {page.url}." + + +def _act_reload(page, selector: str, value: str) -> str: + page.reload(wait_until="load", timeout=NAVIGATION_TIMEOUT_MS) + + return f"Reloaded {page.url}." + + +_ACTIONS = { + "click": _act_click, + "fill": _act_fill, + "press": _act_press, + "hover": _act_hover, + "select": _act_select, + "scroll": _act_scroll, + "wait": _act_wait, + "eval": _act_eval, + "back": _act_back, + "reload": _act_reload, +} + +# Everything the model may put in the action argument. `close` is handled +# by the tool itself, since it ends the session rather than touching the +# page. +ACTIONS = (*sorted(_ACTIONS), "close") + +MAX_EVAL_RESULT = 500 + + +def _short(result) -> str: + """A JavaScript result the model can read without drowning in it.""" + + text = " ".join(str(result).split()) + + if not text: + return "nothing" + + if len(text) > MAX_EVAL_RESULT: + return text[:MAX_EVAL_RESULT] + " ... (truncated)" + + return text diff --git a/flash/system_prompt.txt b/flash/system_prompt.txt index 95da946..8b23c6e 100644 --- a/flash/system_prompt.txt +++ b/flash/system_prompt.txt @@ -30,6 +30,13 @@ Fix and re-check rather than narrating. If the render is wrong, edit the file an Be honest about what you looked at. Name the widths you captured and say `full_page` if you used it. A screenshot shows one static frame, so it cannot tell you about hover, focus, scroll behavior, or anything that needs a click, and you never claim otherwise. If the tool reports that the model has no vision support or that Chromium is missing, relay the exact command it names, say the page is unverified, and never describe a render you did not see. +== Writing long files (write a piece at a time) == +Your reply has a hard token limit, and a `write` call is part of that reply, so a file whose full contents don't fit is cut off mid-call: the tool call is malformed or lands truncated, and you lose the whole thing. Never try to emit a long file in one `write`. As a rule of thumb, keep each call to roughly 80 lines or 3000 characters of content; anything longer gets split. + +Build the file in pieces instead. Call `write` with the first piece and no `append` (this replaces the file, so you start clean), then call `write` again with `append=true` for each following piece, in order, until the file is complete. Content is written byte-for-byte, so end every piece with a newline, or start the next one with a newline, and never resend a piece you already appended, appending twice duplicates it. Plan the split at a natural boundary, the end of a function, a class, a section, a paragraph, not in the middle of a line or a string literal, and keep going in the same turn until you've written the last piece; a half-written file is worse than none, and your turn ends the moment you reply with text. + +The same applies when you're editing rather than creating: `write` without `append` replaces the whole file, so for a large existing file don't read it and echo the whole thing back with one line changed. Prefer a targeted `shell` edit for a small change to a big file, or rebuild it in pieces as above when the change is extensive. If a `write` call ever comes back truncated or malformed, don't retry the same oversized call, split it smaller and continue with `append=true`. + == Code review == When asked to review code (a diff, a PR, a branch, a directory, "review my changes"), first pin down the exact scope with `shell`: `git diff`, `git diff ...`, or `git show` for a specific commit; a plain recursive listing plus reads when there's no history to diff against. Read every changed file in full surrounding context, not just the diff hunk, a hunk without the function it lives in can hide broken control flow, a missed caller, or a signature change that isn't obviously wrong in isolation. When a change touches a shared function, type, or config, grep the rest of the codebase for its other call sites with `shell` before deciding the change is safe. diff --git a/flash/tools.py b/flash/tools.py index 58feb84..002cba2 100644 --- a/flash/tools.py +++ b/flash/tools.py @@ -17,7 +17,21 @@ from ddgs import DDGS from rich.text import Text -from .browser import capture, resolve_target +from .browser import ( + ACTIONS, + MAX_ELEMENTS, + NO_PAGE, + capture, + close_session, + resolve_target, +) +from .browser import drain_problems as page_problems +from .browser import elements as page_elements +from .browser import interact as browser_interact +from .browser import is_open as page_is_open +from .browser import open_page as browser_open +from .browser import snapshot as page_snapshot +from .browser import where as page_where from .documents import extract_document_text, is_document_path from .images import resolve_image_path from .memory import add_memory, forget_memory, search_memory @@ -56,7 +70,10 @@ heredocs, or Set-Content. It needs no quoting or escaping and works the same on every platform, so shell quoting can never corrupt the content. It replaces the whole file, so read the file first when editing one, and - pass back the complete new contents. + pass back the complete new contents. Your reply has a token limit, so a + long file does not fit in one call: write the first part, then call + write again with append=true for each following part, about 80 lines + at a time, until the file is finished. When searching for recent information, use the web_search tool. When you need to know the user's operating system, use the get_os tool. To think or plan mid-task without ending your turn, use the reason tool. @@ -76,6 +93,21 @@ errors the page threw, which is what usually explains a blank section, so read those before changing any CSS. Serve the page over HTTP with shell first if it needs fetch or ES modules, which file:// blocks. +To click a button, fill in a form, or work out why a page misbehaves, + open it with the open_page tool and then drive it with the interact + tool, one action per call: click, fill, press, hover, select, scroll, + wait, eval, back, reload, close. The browser stays open between calls, + so the page keeps whatever state your last action put it in. Each call + answers with the page's address, up to {MAX_ELEMENTS} numbered elements + you can act on, and the JavaScript errors the page threw, and attaches + a fresh screenshot, so you see the result of every action instead of + guessing it. Act on an element by the number beside it; a CSS selector + or the visible text works too. Those numbers are handed out again after + every call, so use the newest list, never one from earlier in the + conversation. The eval action runs JavaScript on the live page and + returns the result, which is the quickest way to check state a picture + cannot show, such as what a handler stored or what a value really is. + Close the browser with the close action once the page is working. To save a durable fact or preference for future sessions, use the remember tool. To check saved memory, use the recall tool with a specific phrase; it does not return everything for a blank search. To delete one saved @@ -579,10 +611,25 @@ def _diff_preview(old_text: str, new_text: str, name: str) -> tuple[ return body[:MAX_DIFF_PREVIEW_LINES], omitted, additions, removals -def write_tool(path: str, content: str) -> str: +def _read_exact(file_path: Path) -> Union[str, None]: # noqa: UP007 + """The file's text exactly as it sits on disk, or None if unreadable.""" + + # newline="" keeps the line endings exactly as they are on disk, + # which is the whole point of reading it again here. Path.read_text + # only learned that argument in 3.13, and Flash supports 3.10. + try: + with open(file_path, encoding="utf-8", newline="") as handle: + return handle.read() + except (OSError, UnicodeDecodeError): + return None + + +def write_tool(path: str, content: str, append: Any = False) -> str: """Tool to write a text file, showing a diff and asking to confirm.""" - tool_line(f"Write({path})") + adding = bool(append) + + tool_line(f"Write({path}, append)" if adding else f"Write({path})") file_path = Path(path).expanduser() if file_path.is_dir(): @@ -600,8 +647,18 @@ def write_tool(path: str, content: str) -> str: else: old_text = "" + # An append is confirmed as the whole file it will produce, so the + # user sees the new lines in place rather than a fragment out of + # context. The exact text matters: whether the file already ends in + # a newline decides whether the first added line joins the last one. + if adding and existed: + exact = _read_exact(file_path) + new_text = (old_text if exact is None else exact) + content + else: + new_text = content + preview, omitted, additions, removals = _diff_preview( - old_text, content, file_path.name + old_text, new_text, file_path.name ) if not existed: @@ -621,7 +678,10 @@ def write_tool(path: str, content: str) -> str: notify_needs_input() prompt = Text(f" {BRANCH} ", style=DIM) - prompt.append("Write this file? ", style=DIM) + prompt.append( + "Append to this file? " if adding else "Write this file? ", + style=DIM, + ) prompt.append("y", style=f"bold {ACCENT}") prompt.append("/n ", style=DIM) console.print(prompt, end="") @@ -634,7 +694,8 @@ def write_tool(path: str, content: str) -> str: file_path.parent.mkdir(parents=True, exist_ok=True) # newline="" so the model's content lands byte-for-byte, instead of # every \n becoming \r\n on Windows. - with open(file_path, "w", encoding="utf-8", newline="") as handle: + mode = "a" if adding else "w" + with open(file_path, mode, encoding="utf-8", newline="") as handle: handle.write(content) except OSError as exc: result = f"Error: could not write {file_path}: {exc}" @@ -642,6 +703,17 @@ def write_tool(path: str, content: str) -> str: return result written = len(content.splitlines()) + + if adding: + total = len(new_text.splitlines()) + tool_result(f"Appended {written} line{plural(written)}") + + return ( + f"Appended {written} line{plural(written)} to {file_path}, " + f"which now has {total} line{plural(total)}. Append the next " + "piece the same way, or stop here if the file is finished." + ) + verb = "Wrote" if existed else "Created" tool_result(f"{verb} {written} line{plural(written)}") return f"{verb} {written} line{plural(written)} to {file_path}" @@ -912,6 +984,166 @@ def screenshot( return result +def _page_report(headline: str, *, full_page: bool = False) -> str: + """Show the model the page it just acted on. + + Every open_page and interact call ends here, because an action the + model cannot see the result of is an action it has to guess about: a + picture when the model has eyes, the elements it can act on next, and + whatever the page complained about while doing it. + """ + + global _screenshot_count + + lines = [headline] + + url, title = page_where() + if url: + lines.append(f"Page: {title or 'untitled'} - {url}") + + if model_sees_images(OLLAMA_HOST, MODEL_NAME): + _screenshot_count += 1 + out = Path(SCRATCH_DIR) / f"page-{_screenshot_count}.png" + why = page_snapshot(out, full_page=bool(full_page)) + + if why: + lines.append(f"No screenshot of the page: {why}") + tool_result(why, style=WARN) + else: + data = out.read_bytes() + _pending_images.append(data) + kilobytes = max(1, round(len(data) / 1024)) + tool_result(f"{out.name} ({kilobytes} KB)") + lines.append( + "A screenshot of the page as it stands is attached to this " + "tool result, so judge it from what you can see there." + ) + else: + lines.append( + f"The active model ({MODEL_NAME}) has no vision, so there is no " + "screenshot. Work from the element list and from eval." + ) + + found, why = page_elements() + if why: + lines.append(f"Could not list the page's elements: {why}") + elif found: + lines.append( + "Things you can act on now (pass the number as the selector):" + ) + lines.extend(found) + else: + lines.append("Nothing on this page can be clicked or typed into.") + + problems = page_problems() + if problems: + for problem in problems[:MAX_PAGE_PROBLEMS]: + tool_result(problem, style=WARN) + + shown = problems[:MAX_PAGE_PROBLEMS] + extra = len(problems) - len(shown) + lines.append( + f"The page reported {len(problems)} error{plural(len(problems))}, " + "which is usually what explains anything that looks wrong:" + ) + lines.extend(f"- {problem}" for problem in shown) + + if extra: + lines.append(f"- and {extra} more") + + return "\n".join(lines) + + +def open_page( + target: str, + width: Any = DEFAULT_SCREENSHOT_WIDTH, + height: Any = DEFAULT_SCREENSHOT_HEIGHT, + wait_ms: Any = DEFAULT_SCREENSHOT_WAIT_MS, +) -> str: + """Open a page in a browser that stays open to be clicked through.""" + + view_width = _clamp(width, MIN_SCREENSHOT_SIDE, MAX_SCREENSHOT_SIDE, + DEFAULT_SCREENSHOT_WIDTH) + view_height = _clamp(height, MIN_SCREENSHOT_SIDE, + MAX_SCREENSHOT_SIDE, DEFAULT_SCREENSHOT_HEIGHT) + settle_ms = _clamp(wait_ms, 0, MAX_SCREENSHOT_WAIT_MS, + DEFAULT_SCREENSHOT_WAIT_MS) + + shape = f"{view_width}x{view_height}" + tool_line(f"OpenPage({target}, {shape})") + + url, why = resolve_target(target) + if url is None: + result = f"Error: {why}" + tool_result(result, style=ERROR) + return result + + why = browser_open( + url, + width=view_width, + height=view_height, + wait_ms=settle_ms, + ) + + if why: + result = f"Error: {why}" + tool_result(result, style=ERROR) + return result + + return _page_report( + f"Opened {url} at {shape}. The browser stays open, so use the " + "interact tool to click, type, or run JavaScript on this page, and " + "close it when you are done." + ) + + +def interact( + action: str, + selector: str = "", + value: str = "", + wait_ms: Any = 0, + full_page: Any = False, +) -> str: + """Act on the page the browser already has open.""" + + action = str(action).strip().lower() + selector = str(selector or "") + value = "" if value is None else str(value) + + label = f"{action} {selector}".strip() + tool_line(f"Interact({label})") + + if action == "close": + result = ( + "Closed the browser." + if close_session() + else "There was no browser open." + ) + tool_result(result) + return result + + if not page_is_open(): + result = f"Error: {NO_PAGE}" + tool_result(result, style=ERROR) + return result + + note, why = browser_interact( + action, + selector=selector, + value=value, + wait_ms=_clamp(wait_ms, 0, MAX_SCREENSHOT_WAIT_MS, 0), + ) + + if why: + # A failed action leaves the page as it was, so the model still + # needs to see it to work out what went wrong. + result = _page_report(f"That did not work: {why}") + tool_result(why, style=ERROR) + return result + + return _page_report(note, full_page=bool(full_page)) + + # Tool schema expected by Ollama function calling (OpenAI-style). tools = [ { @@ -1064,12 +1296,15 @@ def screenshot( "function": { "name": "write", "description": ( - "Write a text file, replacing it if it exists. The user " - "sees a diff and confirms before anything is written. " - "Cross-platform and needs no quoting or escaping; prefer " - "this over shell redirection or heredocs for every file " - "you create or change. Read the file first when editing " - "one, since this replaces the whole file." + "Write a text file, replacing it if it exists, or add to " + "the end of one with append. The user sees a diff and " + "confirms before anything is written. Cross-platform and " + "needs no quoting or escaping; prefer this over shell " + "redirection or heredocs for every file you create or " + "change. Read the file first when editing one, since " + "without append this replaces the whole file. A long file " + "will not fit in one call, so write the first part, then " + "append the rest a piece at a time." ), "parameters": { "type": "object", @@ -1085,7 +1320,20 @@ def screenshot( "type": "string", "description": ( "The file's full new contents, exactly as it " - "should land on disk." + "should land on disk, or the piece to add to " + "the end of it when append is true." + ), + }, + "append": { + "type": "boolean", + "description": ( + "Add content to the end of the file instead of " + "replacing it. Use it to build a file that is " + "too long for one call, one piece per call, " + "and to continue an unfinished one. It is " + "written exactly as given, so start the piece " + "with a newline if the last one did not end " + "with one." ), }, }, @@ -1189,6 +1437,139 @@ def screenshot( }, }, }, + { + "type": "function", + "function": { + "name": "open_page", + "description": ( + "Open a local .html file or a URL in a real browser that " + "stays open, so you can then click, type, and debug your " + "way through the page with the interact tool. The result " + "shows the page's address, a numbered list of everything " + "that can be clicked or typed into, and any JavaScript " + "errors it threw, with a screenshot attached. Use this " + "instead of screenshot whenever the page has buttons, a " + "form, or behaviour to check; screenshot is only a still " + "picture." + ), + "parameters": { + "type": "object", + "properties": { + "target": { + "type": "string", + "description": ( + "Path to a local page, e.g. './index.html', or " + "a URL, e.g. 'http://localhost:8000'." + ), + }, + "width": { + "type": "integer", + "description": ( + "Viewport width in pixels. Defaults to " + f"{DEFAULT_SCREENSHOT_WIDTH}. Use 375 to work " + "through the page as a phone would show it." + ), + "minimum": MIN_SCREENSHOT_SIDE, + "maximum": MAX_SCREENSHOT_SIDE, + }, + "height": { + "type": "integer", + "description": ( + "Viewport height in pixels. Defaults to " + f"{DEFAULT_SCREENSHOT_HEIGHT}." + ), + "minimum": MIN_SCREENSHOT_SIDE, + "maximum": MAX_SCREENSHOT_SIDE, + }, + "wait_ms": { + "type": "integer", + "description": ( + "Milliseconds to let the page load before " + "looking at it. Defaults to " + f"{DEFAULT_SCREENSHOT_WAIT_MS}." + ), + "minimum": 0, + "maximum": MAX_SCREENSHOT_WAIT_MS, + }, + }, + "required": ["target"], + }, + }, + }, + { + "type": "function", + "function": { + "name": "interact", + "description": ( + "Do one thing to the page open_page opened, then look at " + "the result: click a button, fill a field, press a key, " + "choose an option, scroll, wait for something to appear, " + "or run JavaScript against the live page. The page keeps " + "its state between calls, so work through a flow one call " + "at a time. Every call reports where the page is now, its " + "numbered elements, and the errors it threw, with a " + "screenshot attached, so this is how you debug what a page " + "actually does rather than what its source says." + ), + "parameters": { + "type": "object", + "properties": { + "action": { + "type": "string", + "description": ( + "What to do: 'click', 'fill' (type value into " + "a field), 'press' (send a key such as Enter or " + "Tab), 'hover', 'select' (choose value in a " + "dropdown), 'scroll', 'wait', 'eval' (run the " + "JavaScript in value and return its result), " + "'back', 'reload', or 'close' (shut the " + "browser when you are done)." + ), + "enum": list(ACTIONS), + }, + "selector": { + "type": "string", + "description": ( + "Which element to act on: the number shown " + "next to it in the last element list (simplest " + "and safest), a CSS selector, or the visible " + "text on it. The numbers are handed out again " + "after every call, so always use the newest " + "list. Leave it out for eval, back, reload, " + "close, and for a scroll of the whole page." + ), + }, + "value": { + "type": "string", + "description": ( + "The text to type for fill, the key for press, " + "the option for select, the JavaScript for " + "eval, or 'top', 'bottom', or a number of " + "pixels for scroll." + ), + }, + "wait_ms": { + "type": "integer", + "description": ( + "Extra milliseconds to wait after the action " + "before looking, for a page that animates or " + "fetches in response to it." + ), + "minimum": 0, + "maximum": MAX_SCREENSHOT_WAIT_MS, + }, + "full_page": { + "type": "boolean", + "description": ( + "Photograph the whole scrollable page instead " + "of just the viewport." + ), + }, + }, + "required": ["action"], + }, + }, + }, { "type": "function", "function": { @@ -1338,6 +1719,8 @@ def screenshot( "write": write_tool, "view_image": view_image, "screenshot": screenshot, + "open_page": open_page, + "interact": interact, "web_search": web_search, "get_os": get_os, "reason": reason, diff --git a/flash/updater.py b/flash/updater.py index 38233ed..0ce084a 100644 --- a/flash/updater.py +++ b/flash/updater.py @@ -13,7 +13,6 @@ import subprocess # nosec B404 import tempfile import urllib.request -from pathlib import Path from typing import Union from .version import ( @@ -63,13 +62,6 @@ def check_for_update() -> Union[str, None]: # noqa: UP007, RUF100 return latest if latest and is_newer(latest) else None -def is_pipx_install() -> bool: - """True if the running `flash` command lives in a pipx venv.""" - - exe = shutil.which("flash") - return bool(exe) and "pipx" in Path(exe or "").resolve().as_posix() - - def perform_update() -> tuple[bool, str]: """Reinstall Flash from the latest `main` branch. @@ -90,14 +82,12 @@ def perform_update() -> tuple[bool, str]: f"command:\n\n```\n{reinstall}\n```" ) - if not is_pipx_install(): - return False, ( - "This doesn't look like a pipx install. If you cloned the " - "repo manually, update it with `git pull` instead." - ) - tmp_dir = tempfile.mkdtemp(prefix="flash-update-") try: + subprocess.run( # nosec B603 B607 + ["pipx", "uninstall", "flash"], + check=True, capture_output=True, text=True, + ) subprocess.run( # nosec B603 B607 ["git", "clone", "--depth", "1", REPO_URL, tmp_dir], check=True, capture_output=True, text=True, diff --git a/flash/version.py b/flash/version.py index be870cb..eadfc2c 100644 --- a/flash/version.py +++ b/flash/version.py @@ -1,4 +1,4 @@ -__version__ = "0.3.2" +__version__ = "0.3.3" REPO = "Natuworkguy/Flash" REPO_URL = f"https://github.com/{REPO}" diff --git a/tests/test_tools.py b/tests/test_tools.py index 6f4cedf..1ad0885 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -2,10 +2,14 @@ import subprocess # nosec B404 +import pytest + from flash import browser, images, tools from flash.tools import ( glob_tool, grep_tool, + interact, + open_page, read_tool, screenshot, shell_tool, @@ -372,3 +376,221 @@ def test_diff_preview_counts_changes(): assert omitted == 0 # nosec B101 assert "-b" in preview # nosec B101 assert "+B" in preview # nosec B101 + + +def _fake_page(monkeypatch, tmp_path, *, elements=(), problems=()): + """Stand in for a live Chromium, writing the screenshot it promises.""" + + monkeypatch.setattr(tools, "MODEL_NAME", "") + monkeypatch.setattr(tools, "SCRATCH_DIR", str(tmp_path)) + monkeypatch.setattr(tools, "model_sees_images", lambda *_: True) + monkeypatch.setattr(tools, "page_is_open", lambda: True) + monkeypatch.setattr( + tools, "page_where", lambda: ("http://localhost/page", "Demo") + ) + monkeypatch.setattr(tools, "page_elements", lambda: (list(elements), "")) + monkeypatch.setattr(tools, "page_problems", lambda: list(problems)) + + def snapshot(out, **_kwargs): + out.write_bytes(b"png bytes") + + return "" + + monkeypatch.setattr(tools, "page_snapshot", snapshot) + + +def test_open_page_shows_the_page_and_what_it_can_click(tmp_path, monkeypatch): + take_pending_images() + _fake_page( + monkeypatch, + tmp_path, + elements=['[1] button "Add one"'], + problems=["page error: boom is not defined"], + ) + monkeypatch.setattr(tools, "browser_open", lambda *_a, **_k: "") + page = tmp_path / "index.html" + page.write_text("") + + result = open_page(str(page)) + + assert "Opened file://" in result # nosec B101 + assert '[1] button "Add one"' in result # nosec B101 + assert "boom is not defined" in result # nosec B101 + assert take_pending_images() == [b"png bytes"] # nosec B101 + + +def test_open_page_surfaces_a_browser_that_will_not_start( + tmp_path, monkeypatch +): + take_pending_images() + _fake_page(monkeypatch, tmp_path) + monkeypatch.setattr( + tools, "browser_open", lambda *_a, **_k: browser.BROWSER_HINT + ) + page = tmp_path / "index.html" + page.write_text("

hi

") + + result = open_page(str(page)) + + assert "playwright install chromium" in result # nosec B101 + assert take_pending_images() == [] # nosec B101 + + +def test_interact_needs_a_page_to_be_open(tmp_path, monkeypatch): + take_pending_images() + _fake_page(monkeypatch, tmp_path) + monkeypatch.setattr(tools, "page_is_open", lambda: False) + + result = interact("click", selector="1") + + assert "No page is open" in result # nosec B101 + assert take_pending_images() == [] # nosec B101 + + +def test_interact_shows_the_page_after_every_action(tmp_path, monkeypatch): + take_pending_images() + _fake_page(monkeypatch, tmp_path, elements=['[1] button "Add one"']) + monkeypatch.setattr( + tools, "browser_interact", lambda *_a, **_k: ("Clicked 1.", "") + ) + + result = interact("click", selector="1") + + assert "Clicked 1." in result # nosec B101 + assert '[1] button "Add one"' in result # nosec B101 + assert take_pending_images() == [b"png bytes"] # nosec B101 + + +def test_interact_still_shows_the_page_when_the_action_fails( + tmp_path, monkeypatch +): + take_pending_images() + _fake_page(monkeypatch, tmp_path, elements=['[1] button "Add one"']) + monkeypatch.setattr( + tools, + "browser_interact", + lambda *_a, **_k: ("", "Nothing on the page matches 'ghost'."), + ) + + result = interact("click", selector="ghost") + + assert "did not work" in result # nosec B101 + assert '[1] button "Add one"' in result # nosec B101 + assert take_pending_images() == [b"png bytes"] # nosec B101 + + +def test_interact_close_shuts_the_browser(tmp_path, monkeypatch): + take_pending_images() + _fake_page(monkeypatch, tmp_path) + closed = [] + monkeypatch.setattr(tools, "close_session", lambda: closed.append(1) or 1) + + result = interact("close") + + assert closed == [1] # nosec B101 + assert "Closed the browser" in result # nosec B101 + assert take_pending_images() == [] # nosec B101 + + +class _FakeLocator: + """The little of Playwright's locator that `_locate` leans on.""" + + def __init__(self, count): + self._count = count + self.first = self + + def count(self): + return self._count + + +class _FakePage: + """A page where only the given selectors match anything.""" + + def __init__(self, matches): + self.matches = matches + self.asked = [] + + def locator(self, selector): + self.asked.append(selector) + + return _FakeLocator(self.matches.get(selector, 0)) + + def get_by_text(self, text): + return _FakeLocator(self.matches.get(f"text={text}", 0)) + + +def test_locate_takes_an_element_number(): + page = _FakePage({'[data-flash-id="3"]': 1}) + + assert browser._locate(page, "3") is not None # nosec B101 + + +def test_locate_falls_back_from_a_selector_to_the_visible_text(): + page = _FakePage({"text=Sign in": 1}) + + assert browser._locate(page, "Sign in") is not None # nosec B101 + assert "Sign in" in page.asked # nosec B101 + + +def test_locate_explains_a_stale_element_number(): + page = _FakePage({}) + + with pytest.raises(browser.PageProblem) as caught: + browser._locate(page, "9") + + assert "no element 9" in str(caught.value) # nosec B101 + + +def test_interact_turns_down_an_action_it_does_not_have(monkeypatch): + monkeypatch.setattr(browser, "is_open", lambda: True) + + _, why = browser.interact("frobnicate") + + assert "Unknown action" in why # nosec B101 + + +def test_write_tool_appends_instead_of_replacing(tmp_path, monkeypatch): + monkeypatch.setattr(tools, "NO_COMMAND_CONFIRMATION", True) + target = tmp_path / "long.py" + + write_tool(str(target), "first\nsecond\n") + result = write_tool(str(target), "third\n", append=True) + + assert target.read_bytes() == b"first\nsecond\nthird\n" # nosec B101 + assert "Appended 1 line" in result # nosec B101 + assert "now has 3 lines" in result # nosec B101 + + +def test_write_tool_append_creates_a_missing_file(tmp_path, monkeypatch): + monkeypatch.setattr(tools, "NO_COMMAND_CONFIRMATION", True) + target = tmp_path / "new" / "part.txt" + + write_tool(str(target), "start\n", append=True) + + assert target.read_bytes() == b"start\n" # nosec B101 + + +def test_write_tool_append_joins_a_file_with_no_trailing_newline( + tmp_path, monkeypatch +): + monkeypatch.setattr(tools, "NO_COMMAND_CONFIRMATION", True) + target = tmp_path / "seam.txt" + target.write_bytes(b"tail") + + write_tool(str(target), "ing\n", append=True) + + assert target.read_bytes() == b"tailing\n" # nosec B101 + + +def test_write_tool_append_blocked_leaves_the_file_untouched( + tmp_path, monkeypatch +): + monkeypatch.setattr(tools, "NO_COMMAND_CONFIRMATION", False) + monkeypatch.setattr("builtins.input", lambda: "n") + target = tmp_path / "keep.txt" + target.write_bytes(b"original\n") + + result = write_tool(str(target), "more\n", append=True) + + assert target.read_bytes() == b"original\n" # nosec B101 + assert "blocked by user" in result # nosec B101 diff --git a/tests/test_updater.py b/tests/test_updater.py index 1e81f26..8056ef6 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -1,11 +1,6 @@ # pylint: disable=C0114,C0115,C0116 -from flash.updater import ( - check_for_update, - fetch_latest_version, - is_newer, - is_pipx_install, -) +from flash.updater import check_for_update, fetch_latest_version, is_newer from flash.version import __version__ @@ -40,8 +35,3 @@ def test_check_for_update_available(monkeypatch): "flash.updater.fetch_latest_version", lambda: "99.0.0" ) assert check_for_update() == "99.0.0" # nosec B101 - - -def test_is_pipx_install_when_flash_not_on_path(monkeypatch): - monkeypatch.setattr("shutil.which", lambda _name: None) - assert not is_pipx_install() # nosec B101