From 7e846ca062ca251f3173f6bb541af8b376f0b643 Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Tue, 28 Jul 2026 16:23:45 -0400 Subject: [PATCH] frontend-triage: link source files as Searchfox permalinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The triage comment cited source files as bare or backticked repo-relative paths, so a developer reading the bug had to go find the file themselves. Every source-file reference now becomes an inline Markdown link to a revision-pinned searchfox.org URL, so the link keeps pointing at the code the agent actually read after the tree moves on. The agent never handles the revision. It writes `{{searchfox.permalink}}/#` — the same placeholder convention as `{{actions..url}}` — and an action hook on bugzilla.add_comment expands the prefix as the comment is recorded. Expanding at record rather than apply time means the comment waiting in summary.json for review is already clickable, which a prior action's result could not be. A path that is not in the checkout is deliberately not linked: the hook unwraps the link to its backticked label and logs why. Models do invent a plausible path now and then, and a permalink that 404s for whoever clicks it is worse than plain text. The check is a heuristic — the checkout and the pinned revision are hours apart — but it catches the common case. The revision comes from the permalink Searchfox publishes on a file page, not from the checkout's base commit, because Searchfox only serves /rev/ for revisions it has *indexed*, and its index trails the tip of firefox.git by hours (83 commits / ~20h when measured). A link pinned to the checkout 500s for about a day — exactly the window in which someone reads the comment. If no revision can be resolved the placeholder expands to Searchfox's revision-agnostic /source/ prefix: still a working link, just unpinned. Comments are now posted with is_markdown, without which Bugzilla rendered the links — and the existing italic footer — literally. Co-Authored-By: Claude Opus 5 (1M context) --- agents/frontend-triage/README.md | 30 ++- .../hackbot_agents/frontend_triage/agent.py | 51 +++++ .../frontend_triage/prompts/system.md | 6 +- .../frontend_triage/rules/frontend-triage.md | 6 +- .../actions/handlers/bugzilla_handler.py | 27 ++- .../hackbot_runtime/searchfox.py | 184 +++++++++++++++++ .../tests/test_bugzilla_handler.py | 12 +- libs/hackbot-runtime/tests/test_searchfox.py | 194 ++++++++++++++++++ 8 files changed, 494 insertions(+), 16 deletions(-) create mode 100644 libs/hackbot-runtime/hackbot_runtime/searchfox.py create mode 100644 libs/hackbot-runtime/tests/test_searchfox.py diff --git a/agents/frontend-triage/README.md b/agents/frontend-triage/README.md index b567640e5a..3dbcba945f 100644 --- a/agents/frontend-triage/README.md +++ b/agents/frontend-triage/README.md @@ -111,9 +111,37 @@ Two things to know before acting on a plan: the agent pinned a root cause in the code, never whether the fix works — it cannot run anything. Read `high` as "trust the diagnosis, still review the patch." -- **Line numbers are model-asserted and drift.** Trust the files, functions and +- **Line numbers are model-asserted.** The permalink pins the revision, so a + cited line can't drift out from under the link — but the number is still the + model's claim about where the problem is. Trust the files, functions and selectors it names; confirm exact lines against the source. +Source files in the comment are inline Markdown links to **permalinked** +Searchfox URLs (`https://searchfox.org/firefox-main/rev//#`), so +they keep pointing at the code the agent read. + +The agent never handles the revision: it writes +`{{searchfox.permalink}}/#`, the same placeholder convention as +`{{actions..url}}`, and an action hook on `bugzilla.add_comment` expands the +prefix as the comment is recorded — so what lands in `summary.json` for review is +already clickable. `agent.py` resolves the revision once per run with +`hackbot_runtime.searchfox.resolve_index_revision()`. + +A path the agent names that isn't in the checkout is **not** linked: the hook +unwraps the Markdown link to its backticked label and logs a warning. Models do +occasionally invent a plausible path, and a permalink that 404s for whoever +clicks it is worse than plain text. The check is a heuristic — the checkout and +the pinned revision are hours apart, so a file added in between exists locally +but not on Searchfox — but it catches the common case. + +That reads the permalink Searchfox publishes on a file page, rather than the +checkout's base commit, because Searchfox only serves `/rev/` for revisions +it has **indexed** — and its index trails the tip of `firefox.git` by hours (83 +commits / ~20h when measured), so a link pinned to the checkout would 500 for +about a day, exactly the window in which someone reads the comment. If the +revision can't be resolved at all, the placeholder expands to Searchfox's +revision-agnostic `/source/` prefix: still a working link, just unpinned. + ## Tuning - **`rules/`** is the main behavior dial. `scoping.md` decides what gets skipped; diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py b/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py index 4198f70afd..5e5a6e7311 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py @@ -32,6 +32,14 @@ from hackbot_runtime.actions import ACTIONS_SERVER_NAME from hackbot_runtime.actions.claude_sdk import actions_server_for, actions_to_tool_names from hackbot_runtime.claude import Reporter +from hackbot_runtime.searchfox import ( + PLACEHOLDER as SEARCHFOX_PLACEHOLDER, +) +from hackbot_runtime.searchfox import ( + permalink_hook, + permalink_prefix, + resolve_index_revision, +) from searchfox import AsyncSearchfoxClient from .config import ( @@ -67,12 +75,40 @@ class FrontendTriageResult(HackbotAgentResult): result: str | None = None +# How to cite a source file in the recorded comment. Injected into system.md +# rather than written there literally, because the template is run through +# str.format and the placeholder's braces would need doubling twice over in the +# prompt file; a substituted value passes through untouched. +_EXAMPLE_PATH = "browser/components/tabbrowser/content/tabgroup.js" +SEARCHFOX_LINKS_PROMPT = ( + "Every source file you reference in your Bugzilla comment must be an inline " + f"Markdown link built from the `{SEARCHFOX_PLACEHOLDER}` placeholder, which " + "is expanded into a revision-pinned Searchfox URL when your comment is " + "recorded:\n\n" + f" [{_EXAMPLE_PATH}]({SEARCHFOX_PLACEHOLDER}/{_EXAMPLE_PATH})\n\n" + "- Write the placeholder **literally**. Do not put a revision, `tip` or " + "`HEAD` in it, and do not write a searchfox.org URL yourself — you do not " + "know which revision is being linked.\n" + "- After it, give the repo-relative path, plus a line anchor when you know " + "the line: `#1234`, or `#1234-1250` for a range. No `L` prefix.\n" + "- Use the path as the link text, without backticks — backticked text does " + "not render as a link.\n" + "- Leave the paths in the trailing ```json plan block as **bare paths** — " + "that block is parsed by a downstream tool, and a link there would corrupt " + "it.\n" + "- Only cite a path you have confirmed exists (Read/Glob it, or take it from " + "a Searchfox result). A path that is not in the checkout is stripped back to " + "plain text rather than linked, so guessing costs you the link." +) + + def load_system_prompt(rules_dir: Path, extra: str) -> str: tmpl = (HERE / "prompts" / "system.md").read_text() return tmpl.format( rules_dir=str(rules_dir.resolve()), extra_instructions=extra or "(none)", + searchfox_links=SEARCHFOX_LINKS_PROMPT, ) @@ -183,6 +219,21 @@ async def run_frontend_triage( ) vcs_server = build_sdk_server("mozilla_vcs", MozillaVcsContext(), mozilla_vcs.TOOLS) + # Pin every source-file reference in the recorded comment to one revision. + # The agent writes a placeholder (see SEARCHFOX_LINKS_PROMPT) that this hook + # expands as the comment is recorded, so the agent never handles the SHA. An + # unresolvable revision degrades to revision-agnostic /source/ links. + searchfox_rev = await resolve_index_revision() + if not searchfox_rev: + print( + "[frontend_triage] no searchfox revision; linking tip-of-tree", + file=sys.stderr, + ) + actions_recorder.add_hook( + "bugzilla.add_comment", + permalink_hook(permalink_prefix(searchfox_rev), source_repo.resolve()), + ) + system_prompt = load_system_prompt(rules_dir, instructions) options = ClaudeAgentOptions( diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md index 03ca87798b..5fb85d275d 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md @@ -32,7 +32,11 @@ Your working directory is the Firefox source repository. You have Read, Grep, Gl **Always look for an existing test that exercises the affected area** (browser-chrome mochitests usually live in a component's `tests/browser/` directory; also check `tests/`/`test/` and xpcshell tests). Record what you find in the `relevant_tests` field — it is the downstream executor's verification anchor. If you searched and there is no covering test, say so (empty `relevant_tests`). -When you reference a cause or a fix target, cite concrete paths (and ideally functions/selectors), e.g. `browser/components/tabbrowser/content/tabgroup.js`. +When you reference a cause or a fix target, cite concrete paths (and ideally functions/selectors), e.g. `browser/components/tabbrowser/content/tabgroup.js`. In your Bugzilla comment those paths must be Searchfox permalinks — see **Linking source files** below. + +# Linking source files + +{searchfox_links} # Code-search & history tools diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/rules/frontend-triage.md b/agents/frontend-triage/hackbot_agents/frontend_triage/rules/frontend-triage.md index 93621bd150..cef0da309c 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/rules/frontend-triage.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/rules/frontend-triage.md @@ -29,8 +29,10 @@ ruleset does not apply — note that and stop. ## Comment Record a single brief comment (a few sentences) with: the suspected root cause, -the target file(s), and the proposed approach. Cite concrete paths. Do not -restate the whole bug. Do not claim the fix is verified — you did not run it. +the target file(s), and the proposed approach. Cite concrete paths, each as an +inline Markdown link to its Searchfox permalink (with a line anchor where you +know the line) per the **Linking source files** section of the system prompt. Do +not restate the whole bug. Do not claim the fix is verified — you did not run it. ## Confidence and field changes diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/handlers/bugzilla_handler.py b/libs/hackbot-runtime/hackbot_runtime/actions/handlers/bugzilla_handler.py index 620364db80..e9650c9e41 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/handlers/bugzilla_handler.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/handlers/bugzilla_handler.py @@ -54,6 +54,21 @@ def _request(method: str, path: str, json_body: dict[str, Any]) -> dict[str, Any return response.json() +def _comment_body(params: dict[str, Any]) -> dict[str, Any]: + """Build the ``comment`` object for a PUT /bug/{id}. + + Agents author their comments in Markdown (paths as Searchfox permalinks, + the italic automated-analysis footer), so ``is_markdown`` is always set -- + without it Bugzilla falls back to the API user's preference and renders the + markup literally. + """ + return { + "body": params["text"], + "is_private": bool(params.get("is_private", False)), + "is_markdown": True, + } + + class UpdateBugHandler: async def apply(self, params: dict[str, Any], ctx: ApplyContext) -> ActionResult: bug_id = params["bug_id"] @@ -75,12 +90,7 @@ async def apply(self, params: dict[str, Any], ctx: ApplyContext) -> ActionResult class AddCommentHandler: async def apply(self, params: dict[str, Any], ctx: ApplyContext) -> ActionResult: bug_id = params["bug_id"] - body = { - "comment": { - "body": params["text"], - "is_private": params.get("is_private", False), - } - } + body = {"comment": _comment_body(params)} try: _request("PUT", f"bug/{bug_id}", body) except Exception as exc: @@ -207,10 +217,7 @@ def merge_resolved(entries: list[tuple[str, dict[str, Any]]]) -> dict[str, Any]: if action_type == "bugzilla.update_bug": changes.update(params.get("changes", {})) elif action_type == "bugzilla.add_comment": - comment = { - "body": params["text"], - "is_private": bool(params.get("is_private", False)), - } + comment = _comment_body(params) combined: dict[str, Any] = {"bug_id": bug_id, "changes": changes} if comment is not None: diff --git a/libs/hackbot-runtime/hackbot_runtime/searchfox.py b/libs/hackbot-runtime/hackbot_runtime/searchfox.py new file mode 100644 index 0000000000..f549f41d68 --- /dev/null +++ b/libs/hackbot-runtime/hackbot_runtime/searchfox.py @@ -0,0 +1,184 @@ +"""Expand Searchfox permalink placeholders in recorded Bugzilla comments. + +A permalink pins a file to a revision, so a source reference in a triage comment +keeps pointing at the code that was actually reasoned about even after the tree +moves on. Rather than have the agent write (and risk mangling) a 40-character +SHA, it writes a placeholder and this module substitutes the prefix: + + [browser/base/content/browser.js#412]({{searchfox.permalink}}/browser/base/content/browser.js#412) + +Mirrors the ``{{actions..url}}`` convention, but expands at *record* time +rather than apply time: unlike a prior action's result, the revision is known +before the agent runs, and expanding early means the comment awaiting review in +``summary.json`` shows the real URLs a human can click. +""" + +from __future__ import annotations + +import logging +import re +from collections.abc import Callable +from pathlib import Path + +log = logging.getLogger(__name__) + +# Searchfox renamed the Firefox tree from "mozilla-central" to "firefox-main" +# when it moved to git; the old name only 301-redirects. +TREE = "firefox-main" + +_BASE_URL = f"https://searchfox.org/{TREE}" +_USER_AGENT = "bugbug-hackbot-runtime" + +PLACEHOLDER = "{{searchfox.permalink}}" +# Tolerate inner whitespace — the agent writes this by hand. +_PLACEHOLDER = r"\{\{\s*searchfox\.permalink\s*\}\}" +# The placeholder as the whole URL of a Markdown link, so an unresolvable target +# can be degraded by unwrapping the link rather than just dropping its href. +_LINKED_PLACEHOLDER_RE = re.compile( + rf"\[(?P