From 08dafc499175b7001cf74456477b2fd6f9748bcc Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Mon, 17 Aug 2026 16:14:34 -0400 Subject: [PATCH 1/5] Make one registry the source of truth for which components are triaged and where they report `d3f0ed05` added `Firefox :: Site Permissions` across six files, and four of those edits were the same component name written into three differently worded prose lists plus a test. Nothing kept the four in step, and the one that matters is invisible when it goes wrong: `channel_for` fails closed, so a component missing from `SLACK_CHANNELS` still gets its comment and severity change applied unattended, with nobody told. `TRIAGE_SCOPE` in config.py now holds each routed component once, as a `ScopedComponent(product, component, area, channel)`. `SLACK_CHANNELS` is derived from it, so notify.py is unchanged -- it keeps its `.strip()`, its no-default `.get()`, and one flat mapping to look up. `render_scope` in agent.py renders the same tuple into a new `Components in scope` section of system.md, which is a `str.format` template already. `channel` is required rather than optional on purpose. An entry without one would be a component under unattended triage with nobody told, which is what `channel_for` failing closed already produces by accident; there is no reason to be able to express it deliberately. So the registry is exactly the routed set, and it should stay in step with bugbot's `TRIAGED_COMPONENTS`, which decides what arrives automatically. The two are identical today. The rendered section is careful about what the list is not, because a list of components in a system prompt invites two expensive misreadings. Read as exhaustive, it declares an in-scope bug out of scope -- the `ecea6ca6` mistake -- so the section says outright that it is not the limit of what gets triaged and names `scoping.md` as what decides scope: any user-facing Firefox defect qualifies, listed or not. Read as a vocabulary, it gets a bug's component adjusted to match, and since the component is also the routing key, notify.py then tells nobody; so it also asks for `product` and `component` verbatim from Bugzilla. Only the enumeration is generated. The per-area guidance under `Source repository` stays hand-authored markdown: the Site Permissions bullet is six clauses of hedging, and moving it into an implicitly-concatenated literal inside a NamedTuple -- reflowed by ruff-format, where `{` needs no doubling but the surrounding system.md does -- would make the authoring worse to save an edit that only happens when a component's code layout is genuinely new. The bullets are also keyed by area, not component. Not named `TRIAGED_COMPONENTS`, even though it currently holds the same pairs as bugbot's tuple of that name. They answer different questions -- bugbot's is what to send, this is where to report -- and they are in separate repos with separate deploys, so they can legitimately differ for a release. One name for both would hide that. Four new tests, and 46 pass. `test_the_channel_belongs_to_the_component` now loops the registry instead of asserting a line per component, so a new component needs no test edit; the invariants a loop cannot express -- whitespace stripping, fail-closed on an unlisted or garbled pair, `Firefox :: History` being nothing at all, two components sharing a channel -- stay written out. `test_the_registry_names_each_component_once` covers the duplicate key that would collapse silently in the derived dict, and `test_every_channel_is_a_channel_name` the missing `#` that becomes `channel_not_found` at apply time, after the Bugzilla writes have landed. `test_the_scope_says_it_is_neither_a_limit_nor_a_vocabulary` covers both misreadings above. `test_every_area_has_prompt_guidance` is the load-bearing one: it fails when an area has no `Source repository` bullet, which is how a component gets triaged with the agent having no idea where its code lives. --- .../hackbot_agents/frontend_triage/agent.py | 41 +++++++++++ .../hackbot_agents/frontend_triage/config.py | 73 ++++++++++++++++--- .../frontend_triage/prompts/system.md | 4 + agents/frontend-triage/tests/test_notify.py | 38 +++++++++- agents/frontend-triage/tests/test_plan.py | 45 ++++++++++++ 5 files changed, 186 insertions(+), 15 deletions(-) diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py b/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py index 2b8ef00f70..4e76a41548 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/agent.py @@ -49,6 +49,8 @@ ENABLED_ACTION_TYPES, MOZILLA_VCS_TOOLS, SEARCHFOX_TOOLS, + TRIAGE_SCOPE, + ScopedComponent, ) from .hooks import add_comment_hook, update_bug_hook @@ -140,6 +142,44 @@ class FrontendTriageResult(HackbotAgentResult): ) +def render_scope(scope: tuple[ScopedComponent, ...] = TRIAGE_SCOPE) -> str: + """Render `config.TRIAGE_SCOPE` as the prompt's component list, grouped by area. + + Generated rather than written into the prompt so that the component list has one + home. The per-area guidance under `Source repository` stays hand-authored: it is + prose about a codebase, and only the enumeration is mechanical. + + Takes the registry as an argument so a test can assert the grouping against a fixed + input rather than against whatever the real scope happens to be today. + """ + by_area: dict[str, list[str]] = {} + for entry in scope: + by_area.setdefault(entry.area, []).append(entry.key) + + lines = [f"- **{area}** — {', '.join(keys)}." for area, keys in by_area.items()] + + return "\n".join( + lines + + [ + "", + # Two failure modes to close off, in order of how much they cost. Reading the + # list as exhaustive gets an in-scope bug declared out of scope, which is the + # `ecea6ca6` mistake. Reading it as a vocabulary gets a component "tidied" to + # match, and since the component is also the routing key, `notify.py` then + # silently tells nobody. + "**This list is not the limit of what you triage.** It is where bugs " + "normally come from, and which team each one reports to. `scoping.md` is " + "what decides scope: any user-facing Firefox defect is in scope, including " + "in a component not named above — triage it normally rather than calling it " + "out of scope for being absent here.", + "", + "It is also **not** a vocabulary for the `product` and `component` fields " + "of your plan. Copy those from Bugzilla verbatim, even when they are not " + "listed above.", + ] + ) + + def load_system_prompt(rules_dir: Path, extra: str) -> str: tmpl = (HERE / "prompts" / "system.md").read_text() @@ -147,6 +187,7 @@ def load_system_prompt(rules_dir: Path, extra: str) -> str: rules_dir=str(rules_dir.resolve()), extra_instructions=extra or "(none)", searchfox_links=SEARCHFOX_LINKS_PROMPT, + triaged_components=render_scope(), ) diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py index df4ac28749..3373d47a6d 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py @@ -1,3 +1,5 @@ +from typing import NamedTuple + # Bugzilla MCP tool names as exposed to the agent (mcp____). BUGZILLA_READ_TOOLS = [ "mcp__bugzilla__search_bugs", @@ -42,24 +44,73 @@ "bugzilla.update_bug", ] -# Where an auto-applied run reports itself, by `" :: "`. A channel -# belongs to the team that owns the component, so the routing does too: a component -# that is not listed sends nothing, since posting one team's triage into another team's -# channel is worse than silence. There is deliberately no default channel. + +class ScopedComponent(NamedTuple): + """A Bugzilla component sent here for triage, and where a finished run reports it.""" + + product: str + component: str + # Which `Source repository` bullet in prompts/system.md describes this component's + # code. `tests/test_plan.py` asserts every area named here has one, so a new area + # cannot be added without the guidance that makes it triageable. + area: str + # Required, because an entry without one would be a component getting unattended + # triage with nobody told -- which is what `channel_for` failing closed produces, + # and not something to be able to express by accident. + channel: str + + @property + def key(self) -> str: + return f"{self.product} :: {self.component}" + + +# The components that are sent here for triage, and the channel that owns each. The +# single source of truth for both: `SLACK_CHANNELS` below is derived from it, and +# `render_scope` in agent.py renders it into the system prompt, so adding a component is +# one entry here rather than the same name written into three prose lists and a test. +# +# This is narrower than what the agent will triage. `rules/scoping.md` puts any +# user-facing Firefox defect in scope, and a bug handed to the agent by hand is triaged +# on that rule whether or not its component is named here -- it just reports to nobody. +# What this tuple decides is routing, and it should stay in step with bugbot's +# `TRIAGED_COMPONENTS`, which decides what arrives automatically. +# +# A channel belongs to the team that owns the component, so the routing does too: a +# component that is not listed sends nothing, since posting one team's triage into +# another team's channel is worse than silence. There is deliberately no default channel. # # `slack.post_message` is left out of `ENABLED_ACTION_TYPES` on purpose. The message is # code (see notify.py), not a model turn, so it goes through the recorder directly and # the agent is never given the tool — it has no say in what is said or where. -SLACK_CHANNELS = { - "Firefox :: New Tab Page": "#hnt-dev-triage", - "Firefox for Android :: History": "#android-core-dev", +# +# Ordered by area, grouped by first appearance — `render_scope` preserves that order, so +# this is also the order the model reads. There is no separate list of areas to keep in +# sync with this one. +TRIAGE_SCOPE = ( + ScopedComponent("Firefox", "New Tab Page", "Desktop frontend", "#hnt-dev-triage"), + ScopedComponent( + "Firefox", "Site Permissions", "Site permissions", "#privacy-team-automation" + ), + ScopedComponent( + "Firefox for Android", "History", "Firefox for Android", "#android-core-dev" + ), # The installer and the updater are triaged by the same team, so two components # share a channel. Keying by product-and-component rather than by channel is what # lets them, without either one having to know about the other. - "Toolkit :: Application Update": "#installer-updater-bug-triage", - "Firefox :: Installer": "#installer-updater-bug-triage", - "Firefox :: Site Permissions": "#privacy-team-automation", -} + ScopedComponent( + "Toolkit", + "Application Update", + "Application updater", + "#installer-updater-bug-triage", + ), + ScopedComponent( + "Firefox", "Installer", "Windows installer", "#installer-updater-bug-triage" + ), +) + +# Where an auto-applied run reports itself, by `" :: "`. Derived, so +# that `notify.py` keeps one flat mapping to look up. +SLACK_CHANNELS = {c.key: c.channel for c in TRIAGE_SCOPE} # What a `bugzilla.update_bug` from this agent may touch. Enforced at record time # by `hooks.update_bug_hook`, so an out-of-bounds change is refused while the agent 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 23db2830d6..8cbd6e3c39 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md @@ -27,6 +27,10 @@ Do not claim to have "verified" or "tested" a fix. You are reasoning from the co Use **only** these tools for accessing Bugzilla, nothing else. +# Components in scope + +{triaged_components} + # Source repository Your working directory is the Firefox source repository — the whole tree, desktop and Android in one checkout. You have Read, Grep, Glob, and Bash (read-only — do not modify files) to inspect it. Use this to localize the bug: find the modules, markup, styling, and prefs (often under `modules/libpref/init/all.js`) that govern the behaviour, and any existing tests that cover the area. diff --git a/agents/frontend-triage/tests/test_notify.py b/agents/frontend-triage/tests/test_notify.py index 318f1c653c..002601337d 100644 --- a/agents/frontend-triage/tests/test_notify.py +++ b/agents/frontend-triage/tests/test_notify.py @@ -10,6 +10,7 @@ FrontendTriageResult, SeverityAssessment, ) +from hackbot_agents.frontend_triage.config import TRIAGE_SCOPE from hackbot_agents.frontend_triage.notify import ( build_message, channel_for, @@ -87,13 +88,36 @@ def test_a_missing_summary_leaves_the_bug_link_alone(): def test_the_channel_belongs_to_the_component(): - assert channel_for("Firefox", "New Tab Page") == "#hnt-dev-triage" - assert channel_for("Firefox for Android", "History") == "#android-core-dev" - assert channel_for("Firefox", "Site Permissions") == "#privacy-team-automation" + # Derived from the registry rather than one assert per component, so adding a + # component is one entry in config.py and no test edit. The invariants a loop + # cannot express are asserted concretely in the tests below. + for entry in TRIAGE_SCOPE: + assert channel_for(entry.product, entry.component) == entry.channel + + +def test_whitespace_around_either_half_is_stripped(): # Surrounding whitespace is the agent's, not Bugzilla's. assert channel_for(" Firefox ", " New Tab Page ") == "#hnt-dev-triage" +def test_the_registry_names_each_component_once(): + # A duplicate key collapses silently in the derived `SLACK_CHANNELS`, so the second + # entry's channel would win with nothing to show it had. That is the failure mode a + # registry has as it grows, and it is invisible in a diff that only adds a line. + keys = [entry.key for entry in TRIAGE_SCOPE] + assert len(keys) == len(set(keys)) + + +def test_every_channel_is_a_channel_name(): + # Slack rejects an unknown channel at apply time, long after the Bugzilla comment + # and severity change have landed, and the run page is the only place it shows. A + # missing `#` or a stray capital is the whole cost of that, so catch it here. + for entry in TRIAGE_SCOPE: + assert entry.channel.startswith("#"), entry.key + assert entry.channel == entry.channel.strip().lower(), entry.key + assert " " not in entry.channel, entry.key + + def test_the_installer_and_the_updater_share_a_channel(): # One team triages both, and a key that is off by a character notifies nobody # rather than failing, so each one is asserted rather than assumed from the other. @@ -105,10 +129,16 @@ def test_the_installer_and_the_updater_share_a_channel(): def test_an_unowned_component_has_no_channel(): # Fails closed rather than falling back to a default: another team's channel is a # worse outcome than silence. + # + # `Address Bar` is a component this agent will triage if handed one -- scoping.md + # puts any user-facing Firefox defect in scope -- but it is not in `TRIAGE_SCOPE`, + # so no team is told. That is the case worth pinning: in scope and unrouted are + # different questions, and only the second one is this function's business. assert channel_for("Firefox", "Address Bar") is None assert channel_for("Core", "New Tab Page") is None # A component name is only owned within its own product: `History` routes to - # #android-core-dev under Firefox for Android and nowhere at all under Firefox. + # #android-core-dev under Firefox for Android and nowhere at all under Firefox -- + # which has no `History` component in the first place, only `Bookmarks & History`. assert channel_for("Firefox", "History") is None assert channel_for("Firefox", None) is None assert channel_for(None, "New Tab Page") is None diff --git a/agents/frontend-triage/tests/test_plan.py b/agents/frontend-triage/tests/test_plan.py index 3f5ddecbae..cf40baca99 100644 --- a/agents/frontend-triage/tests/test_plan.py +++ b/agents/frontend-triage/tests/test_plan.py @@ -11,7 +11,9 @@ may_apply_unattended, parse_confidence, parse_plan, + render_scope, ) +from hackbot_agents.frontend_triage.config import TRIAGE_SCOPE, ScopedComponent def _block(body: str) -> str: @@ -25,6 +27,49 @@ def test_the_system_prompt_renders(): prompt = load_system_prompt(Path("rules"), "") assert '{"add": ["…"]}' in prompt assert "{rules_dir}" not in prompt + assert "{triaged_components}" not in prompt + # The component list reaches the prompt as full routing keys, since a bare component + # name would not say which product it belongs to. + assert "Firefox :: New Tab Page" in prompt + assert "Toolkit :: Application Update" in prompt + + +def test_the_scope_is_grouped_by_area_in_registry_order(): + # Asserted against a fixed registry rather than the real one, so this keeps testing + # the grouping when TRIAGE_SCOPE changes. + scope = ( + ScopedComponent("Firefox", "New Tab Page", "Desktop", "#one"), + ScopedComponent("Toolkit", "Application Update", "Updater", "#two"), + ScopedComponent("Firefox", "Theme", "Desktop", "#one"), + ) + rendered = render_scope(scope) + assert rendered.startswith( + "- **Desktop** — Firefox :: New Tab Page, Firefox :: Theme.\n" + "- **Updater** — Toolkit :: Application Update.\n" + ) + + +def test_the_scope_says_it_is_neither_a_limit_nor_a_vocabulary(): + # Two ways to misread a list of components in a system prompt, both expensive. + # Reading it as exhaustive declares an in-scope bug out of scope, which is the + # mistake ecea6ca6 was fixing. Reading it as a vocabulary gets a component adjusted + # to match, and the component is the Slack routing key, so the notification then + # goes nowhere without failing. + rendered = render_scope() + assert "not the limit" in rendered + assert "verbatim" in rendered + + +def test_every_area_has_prompt_guidance(): + # The registry is what makes a component triaged; this is what makes it triageable. + # `Source repository` carries the per-area code layout, and an area with no bullet + # there means the agent is pointed at a component with no idea where its code lives + # -- which is how a bug gets read as out of scope and skipped. So a new area costs + # two files, visibly, rather than one file plus a prompt nobody remembered. + prompt = load_system_prompt(Path("rules"), "") + source_section = prompt.split("# Source repository", 1)[1] + for area in {entry.area for entry in TRIAGE_SCOPE}: + assert f"**{area}**" in source_section, area def test_confidence_is_normalized(): From 0583ff6b12724f06de4cea08acc552cbdc783dad Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Mon, 17 Aug 2026 16:16:03 -0400 Subject: [PATCH 2/5] Stop writing the component list into the two rulesets and the README, now that the registry renders it The same set was enumerated in three places, in three wordings, with nothing keeping them in step: `rules/frontend-triage.md` named nine desktop components, `rules/scoping.md` named seven overlapping ones, and README.md named eight plus repeated the whole routing table from config.py. `d3f0ed05` had to edit all three to add one component, and the README table is the one that had already drifted into telling the reader to open config.py while also copying it. All three now point at **Components in scope**, which f076cc9e renders from `TRIAGE_SCOPE`. The prose that was doing work stays: scoping.md keeps the install-and-update paragraph, frontend-triage.md keeps everything below its opening sentence, and the README keeps the four area descriptions with the language each implies, since those change on the order of once a year rather than per component. This does change what the model sees, and in the direction we want. The list moves out of `rules/*.md`, which the agent globs and reads only when it judges a ruleset relevant, and into system.md, which is unconditionally in context. frontend-triage.md already deferred upward for the per-area layout, so this follows the existing grain -- deferring the other way, with the system prompt pointing at an optionally-read file for the scope, would not be safe. The deleted lists were wider than `TRIAGE_SCOPE` -- they named Address Bar, Menus, Theme, Session Restore and others that route nowhere -- so the rulesets now say explicitly what those lists only implied by ending in an ellipsis: `scoping.md`'s rule is that any user-facing Firefox defect is in scope, and a component's absence from the prompt's list is not a reason to skip a bug. Without that sentence this commit would have narrowed the agent's effective scope while appearing only to move a list. The README table is replaced by the four things about routing that are not obvious from reading the registry, plus one worked example: that the key is the component rather than the team, that `TRIAGE_SCOPE` is narrower than what the agent will triage and should stay in step with bugbot's `TRIAGED_COMPONENTS`, that there is no default channel, and that a garbled `product`/`component` matches nothing. The cost is that a reader who wants to know where one component routes now opens config.py. I considered keeping the table behind a test that regexes its rows back out and compares them to `SLACK_CHANNELS`, and decided a six-row list did not earn the machinery -- and prettier realigns every row when the widest cell changes, so "add one row" is a six-row diff either way. 46 tests pass unchanged; none of them read the markdown. --- agents/frontend-triage/README.md | 62 +++++++++++-------- .../frontend_triage/rules/frontend-triage.md | 13 ++-- .../frontend_triage/rules/scoping.md | 10 +-- 3 files changed, 46 insertions(+), 39 deletions(-) diff --git a/agents/frontend-triage/README.md b/agents/frontend-triage/README.md index d33fa01d1e..514a35b056 100644 --- a/agents/frontend-triage/README.md +++ b/agents/frontend-triage/README.md @@ -14,15 +14,20 @@ human (or a downstream execution agent) takes it from there. ## What it triages **Defects in user-facing Firefox** — the kind documented with a screenshot, steps -to reproduce, or a log rather than a stack trace: +to reproduce, or a log rather than a stack trace, across four areas: -- **Desktop frontend**, under `Firefox`: Tabbed Browser (incl. Split View and Tab - Groups), New Tab Page, Address Bar, Menus, Toolbars and Customization, Sidebar, - Site Permissions, Theme. -- **Firefox for Android**: History. Kotlin under `mobile/android/fenix/`. +- **Desktop frontend**, under `Firefox`. JS/JSM modules, CSS, XUL/HTML. +- **Site permissions**, also desktop, but split across the doorhanger, the state, + and a C++ store outside `browser/`. +- **Firefox for Android**. Kotlin under `mobile/android/fenix/` and + `mobile/android/android-components/`. - **Install and update**: `Firefox :: Installer` (NSIS) and `Toolkit :: Application Update` (`.sys.mjs`, IDL, C++). +`TRIAGE_SCOPE` in `config.py` is the component list, one entry per component, +carrying the area and the Slack channel. `prompts/system.md` renders it into the +system prompt and describes each area's code layout. + Install and update bugs are the odd ones out: they arrive as a failure with an error code and an `update.log` or installer log, usually with no steps to reproduce and no screenshot. That is the normal shape of a bug in that area, so @@ -173,22 +178,25 @@ The audience is the team whose bug was just written to by nobody, so only an auto-applied run notifies. A medium or low result wrote nothing to Bugzilla and stays silent, even if someone applies it by hand later. -Routing is `SLACK_CHANNELS` in `config.py`, keyed by `" :: "`: - -| Product :: Component | Channel | -| -------------------------------- | ------------------------------- | -| `Firefox :: New Tab Page` | `#hnt-dev-triage` | -| `Firefox for Android :: History` | `#android-core-dev` | -| `Toolkit :: Application Update` | `#installer-updater-bug-triage` | -| `Firefox :: Installer` | `#installer-updater-bug-triage` | -| `Firefox :: Site Permissions` | `#privacy-team-automation` | - -Two components may share a channel, as the installer and the updater do; the key is -the component, not the team. A component that is not listed notifies nobody; there is -deliberately no default channel, since posting one team's triage into another team's -channel is worse than silence. Product and component come from the agent's -`product`/`component` plan fields, because nothing else carries them out of a run -whose only input is a bug id, so a garbled value matches no team and sends nothing. +Routing is the `channel` on each `TRIAGE_SCOPE` entry in `config.py`, looked up by +`" :: "` through the derived `SLACK_CHANNELS` — so +`ScopedComponent("Firefox", "New Tab Page", "Desktop frontend", "#hnt-dev-triage")` +sends a New Tab Page run to `#hnt-dev-triage`. + +Four things about that which are not obvious from reading the registry: + +- **The key is the component, not the team**, so two components may share a channel, as + the installer and the updater do, without either knowing about the other. +- **A `channel=None` entry is in scope and notifies nobody.** The agent has a ruleset + for it if a human hands it that bug; no team is told. This is not the same as being + outside bugbot's automatic scope — bugbot decides what gets sent, and a pair it sends + whose component has no channel here gets unattended triage in silence. +- **There is deliberately no default channel**, since posting one team's triage into + another team's channel is worse than silence. +- **Product and component come from the agent's `product`/`component` plan fields**, + because nothing else carries them out of a run whose only input is a bug id. A garbled + value matches no team and sends nothing, which is why the system prompt asks for them + verbatim even for components the scope list does not name. `notify.py` builds and records the message; the wording is code, not a model turn, so `slack.post_message` is _not_ in `ENABLED_ACTION_TYPES` and the agent is never @@ -205,12 +213,14 @@ notifies. The run page shows the failed action. `rules/` and `prompts/` both live under `hackbot_agents/frontend_triage/`. - **`rules/`** is the main behavior dial. `scoping.md` decides what gets skipped; - `frontend-triage.md` sets in-scope components, comment content, and the - confidence thresholds for recording an action. The agent globs the directory - and reads only what it judges relevant, so new `.md` files extend it — see - `rules/README.md` for how to author one. + `frontend-triage.md` sets comment content and the confidence thresholds for + recording an action. The agent globs the directory and reads only what it judges + relevant, so new `.md` files extend it — see `rules/README.md` for how to author + one. Neither file lists components; `TRIAGE_SCOPE` in `config.py` does. - **`prompts/system.md`** holds the standing instructions: output format, the - read-only mandate, and when to reach for Searchfox versus reading a file. + read-only mandate, when to reach for Searchfox versus reading a file, and the + per-area code layout. A component in a new area needs a **Source repository** + bullet here, and `tests/test_plan.py` fails until it has one. - **Cost** scales with tool use, not just turns — Searchfox results are token-heavy, so narrowing queries (`path_filter`, a modest `limit`) matters more than `MAX_TURNS` when batching. 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 a972512bbb..09d87a1b70 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 @@ -1,14 +1,11 @@ # User-facing Firefox defect triage These rules apply to **defects in user-facing Firefox** — the desktop frontend, -Firefox for Android, and the Windows installer and application updater. Typical -components: - -- Desktop frontend, all under `Firefox`: `Tabbed Browser`, - `Tabbed Browser: Split View`, `New Tab Page`, `Address Bar`, `Menus`, - `Toolbars and Customization`, `Sidebar`, `Site Permissions`, `Theme`. -- Android: `Firefox for Android :: History`. -- Install and update: `Firefox :: Installer`, `Toolkit :: Application Update`. +Firefox for Android, and the Windows installer and application updater. **Components in +scope** in the system prompt lists the components bugs normally arrive from, grouped by +the area whose code layout **Source repository** describes. Any user-facing Firefox +defect is in scope, though, whether or not its component is on that list; see +`scoping.md`. Desktop and Android bugs here are usually UI/UX papercuts, documented with a **video or screenshot** and steps to reproduce. diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/rules/scoping.md b/agents/frontend-triage/hackbot_agents/frontend_triage/rules/scoping.md index 8ea706488b..4832b08fc5 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/rules/scoping.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/rules/scoping.md @@ -28,11 +28,11 @@ output's `actionable` to `false`, `confidence` to `low`, and `root_cause` to nul ## Proceed normally Everything else — a `defect` in a user-facing Firefox component without the skip signals -above — is in scope. That covers the desktop frontend (Bookmarks & History, New Tab Page, -Session Restore, Sidebar, Site Permissions, Tabbed Browser: Split View / Tab Groups, -Toolbars and Customization, …), Firefox for Android (History, …), and install and update -(`Firefox :: Installer`, `Toolkit :: Application Update`). Continue to the -`frontend-triage` ruleset. +above — is in scope. That covers the desktop frontend, Firefox for Android, and install +and update. **Components in scope** in the system prompt lists the components bugs +normally arrive from, but it is not a limit: a user-facing Firefox defect in a component +that is not on it is still in scope, and being absent from it is not a reason to skip a +bug. Continue to the `frontend-triage` ruleset. An install or update failure reported as an error code and a log, with no steps to reproduce and no screenshot, is an in-scope defect — it is the usual shape of a bug in From bbd05c7162b510bacf27d93e84709fbc1d2094cb Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Mon, 17 Aug 2026 16:17:13 -0400 Subject: [PATCH 3/5] Route Firefox for Android Toolbar and Homepage to #android-core-dev, and tell the agent there are two toolbars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two `ScopedComponent` entries, both `#android-core-dev`, which already receives `Firefox for Android :: History` -- so this needs no Slack work and can ship without waiting on a channel. That is the whole functional change; f076cc9e made the routing tests derive from `TRIAGE_SCOPE`, so there is no test edit and no prose list to update. The prompt is where the work is, because both components sit on a localization trap of the same kind as the stub-versus-full installer the prompt already warns about. There are **two** toolbars: the browser one at `…/fenix/components/toolbar/` and the homepage's own at `…/fenix/home/toolbar/`. Nothing in either name says which surface it serves, so a `Homepage` bug can be localized into a toolbar file and a `Toolbar` bug into the homepage. Under both, android-components carries two generations of the widget -- the Compose `components/compose/browser-toolbar/` and the View-based `components/browser/toolbar/`, with the interface in `concept/toolbar/` and the session wiring in `feature/toolbar/`. A fix planned against the retired implementation reads correct and changes nothing, so the bullet says to confirm which one Fenix builds. Same reason the Fenix-wide Compose migration is now called out on the parent bullet: a screen can have both a `…View.kt` and a `…Composable.kt` with only one live. The homepage is one screen assembled from twelve section subpackages (`topsites/`, `pocket/`, `recenttabs/`, …), so "which section" comes before "which file" and a top-sites bug is not in `Homepage.kt`. `Firefox for Android` also has separate components for several of those sections -- `Top Sites`, `Stories`, `Collections`, `Bookmarks`, `Menu`, `Search` -- so one package is reachable from more than one component, and `Stories` is `home/pocket/` in the tree because the rename never happened. The bullet says to triage the bug under the component it was filed in, which matters twice over: the component also routes the notification. The test-discovery bullet gained the mirror rule, since `HomeFragmentTest.kt` is the easiest thing to find and almost never the right answer, and a note that a Compose surface may only be covered by an `androidTest` UI test -- worth saying rather than reporting no coverage. Volume is modest and lopsided: over the last 90 days, 35 open defects in Homepage of which 24 are staff-filed, against 18 and 2 for Toolbar, so Homepage will be most of this. New Tab Page is 148 and 90 for comparison. `max_triggers` stays at 3. 46 tests pass. bugbot must not start sending these until this is deployed -- without a `SLACK_CHANNELS` entry, `channel_for` fails closed and the comment and severity change land with nobody told. --- .../hackbot_agents/frontend_triage/config.py | 6 ++++++ .../hackbot_agents/frontend_triage/prompts/system.md | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py index 3373d47a6d..523493c1a4 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py @@ -94,6 +94,12 @@ def key(self) -> str: ScopedComponent( "Firefox for Android", "History", "Firefox for Android", "#android-core-dev" ), + ScopedComponent( + "Firefox for Android", "Toolbar", "Firefox for Android", "#android-core-dev" + ), + ScopedComponent( + "Firefox for Android", "Homepage", "Firefox for Android", "#android-core-dev" + ), # The installer and the updater are triaged by the same team, so two components # share a channel. Keying by product-and-component rather than by channel is what # lets them, without either one having to know about the other. 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 8cbd6e3c39..62f1779adf 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md @@ -39,7 +39,9 @@ Where to look, and what you will find there, depends on the bug's component: - **Desktop frontend** — `browser/`, `toolkit/`, and `devtools/`. JS/JSM modules (`.js`, `.mjs`, `.sys.mjs`), CSS, and XUL/HTML. - **Site permissions** — desktop JS, but split across the prompt, the state, and the store, so start by working out which of the three the bug is in. `browser/modules/SitePermissions.sys.mjs` holds the permission state the rest of the frontend reads and writes, including the defaults, the scopes (`SCOPE_PERSISTENT`, `SCOPE_SESSION`, `SCOPE_TEMPORARY`), and the `ALLOW`/`BLOCK`/`PROMPT` states. `browser/modules/PermissionUI.sys.mjs` builds the doorhanger prompts, one subclass per permission type. `browser/actors/WebRTCParent.sys.mjs` handles camera, microphone, and screen sharing, which do **not** go through the generic prompt path and carry their own sharing indicator. The management UI is `browser/components/preferences/dialogs/permissions.js` and `sitePermissions.js`. The backing store is `nsIPermissionManager`, implemented in C++ at `extensions/permissions/PermissionManager.cpp` — that is outside the frontend directories, so "the permission did not stick", "it came back after a restart", and wrong-expiry bugs are localized there and are **not** out of scope for being non-JS. -- **Firefox for Android** — `mobile/android/`, with the Fenix app under `mobile/android/fenix/app/src/main/java/org/mozilla/fenix/` and the reusable components under `mobile/android/android-components/`. This is **Kotlin**, and it is structured as Fragment / Store / Middleware / View rather than as chrome markup plus a script: a `…Fragment.kt` owns the screen, a `…FragmentStore.kt` holds its state and actions, a `…View.kt` or a Compose function renders it, and a `…Middleware.kt` performs side effects. Layouts are Android XML under `mobile/android/fenix/app/src/main/res/layout/`, strings under `res/values/strings.xml`. +- **Firefox for Android** — `mobile/android/`, with the Fenix app under `mobile/android/fenix/app/src/main/java/org/mozilla/fenix/` and the reusable components under `mobile/android/android-components/`. This is **Kotlin**, and it is structured as Fragment / Store / Middleware / View rather than as chrome markup plus a script: a `…Fragment.kt` owns the screen, a `…FragmentStore.kt` holds its state and actions, a `…View.kt` or a Compose function renders it, and a `…Middleware.kt` performs side effects. Layouts are Android XML under `mobile/android/fenix/app/src/main/res/layout/`, strings under `res/values/strings.xml`. Fenix is mid-migration to Jetpack Compose, so a screen may have both a `…View.kt` and a `…Composable.kt` and only one of them is live — check which the Fragment actually builds before planning against either. + - **Android toolbar** — there are **two** toolbars, and a generation of the widget under each. The browser toolbar is `…/fenix/components/toolbar/` (`BrowserToolbarComposable.kt`, `BrowserToolbarMiddleware.kt`, `BrowserNavigationBar.kt`, `ToolbarPosition.kt` for top-versus-bottom, `BottomToolbarContainerView.kt`, `ToolbarsIntegration.kt`); the homepage has its own at `…/fenix/home/toolbar/` (`HomeToolbarComposable.kt`, `FenixHomeToolbar.kt`, `BrowserSimpleToolbar.kt`). So work out which surface the reporter was on first: a `Homepage` bug can localize into a toolbar file and a `Toolbar` bug into the homepage. Underneath both, android-components has the newer Compose widget at `mobile/android/android-components/components/compose/browser-toolbar/` and the older View-based one at `components/browser/toolbar/`, with `components/concept/toolbar/` holding the interface and `components/feature/toolbar/` the session wiring. Confirm which one Fenix builds before citing it — a fix planned against the retired implementation reads correct and changes nothing. + - **Android homepage** — one screen assembled from one package per section, so "which section" comes before "which file". `…/fenix/home/HomeFragment.kt` owns the screen, the Compose UI is under `home/ui/` (`Homepage.kt`, `HomepageHeader.kt`, `SearchBar.kt`, `WallpaperBackground.kt`, `Wordmark.kt`), state is `home/store/HomepageState.kt`, side effects are `home/middleware/`, and the older controller/interactor pair is `home/sessioncontrol/`. Each section is its own subpackage: `topsites/`, `recenttabs/`, `recentsyncedtabs/`, `recentvisits/`, `pocket/`, `bookmarks/`, `collections/`, `setup/`, `sports/`, `mars/`, `logo/`, `privatebrowsing/`. A bug about the top-sites row or the stories feed is localized there, not in `Homepage.kt`. Note also that `Firefox for Android` has separate components for several of these sections — `Top Sites`, `Stories`, `Collections`, `Bookmarks`, `Menu`, `Search` — so the same code can be reached from more than one component, and `Stories` is `home/pocket/` in the tree because nothing was renamed. Triage the bug under the component it was filed in; do not retitle or re-scope it to match. - **Application updater** — `toolkit/mozapps/update/`. `.sys.mjs` modules (`AppUpdater.sys.mjs`, `UpdateService.sys.mjs`, `BackgroundUpdate.sys.mjs`), the XPCOM interfaces in `nsIUpdateService.idl`, and the C++ updater binary under `toolkit/mozapps/update/updater/`. Update behaviour is heavily driven by prefs under `app.update.*` and by the state written to the update directory, so read `common/` for the shared constants and status codes. - **Windows installer** — `browser/installer/windows/nsis/`. This is **NSIS**: `installer.nsi` (the full installer), `stub.nsi` (the small downloader stub), `uninstaller.nsi`, `maintenanceservice_installer.nsi`, and the `.nsh` include files that hold most of the logic. Localized strings live in the `.nsi`/`.properties` files alongside. The packaging manifests are `browser/installer/package-manifest.in` and `browser/installer/allowed-dupes.mn`, and the MSI and MSIX wrappers are in the sibling `msi/` and `msix/` directories. There is no JS here at all. Note which installer the bug is about: the stub and the full installer are separate programs with separate code. @@ -47,7 +49,7 @@ Where to look, and what you will find there, depends on the bug's component: - Desktop: browser-chrome mochitests usually live in a component's `tests/browser/` directory; also check `tests/`/`test/` and xpcshell tests. - Site permissions: the prompts are covered by browser-chrome under `browser/base/content/test/permissions/`, `SitePermissions.sys.mjs` itself by `browser/modules/test/browser/`, and the store by xpcshell under `extensions/permissions/test/`. Name the one that matches the layer you localized to, not whichever you found first. -- Android: Kotlin unit tests under `mobile/android/fenix/app/src/test/java/org/mozilla/fenix/`, and instrumented UI tests under `app/src/androidTest/`. +- Android: Kotlin unit tests under `mobile/android/fenix/app/src/test/java/org/mozilla/fenix/`, and instrumented UI tests under `app/src/androidTest/`. The test tree mirrors the source packages, so name the mirror of the package you localized to — `…/test/java/org/mozilla/fenix/components/toolbar/` for the browser toolbar, `…/fenix/home/topsites/` for a top-sites bug — rather than the screen-level `HomeFragmentTest.kt`. A Compose surface may be covered only by an `androidTest` UI test; say so rather than reporting no coverage. - Updater: `toolkit/mozapps/update/tests/` — xpcshell under `unit_aus_update/`, `unit_background_update/`, and `unit_update_binary/`, browser-chrome under `browser/`, plus `marionette/` and C++ `gtest/`. - Installer: coverage is thin and specific. `browser/installer/windows/nsis/test/xpcshell/test_stub_installer.js` drives `test_stub.nsi` and covers the **stub** installer only; nothing exercises `installer.nsi` or the uninstaller. So for most Installer bugs an empty `relevant_tests` is the correct answer — say that the area is uncovered rather than leaving the reader to wonder whether you looked. From 047194c2d5724c80b1a12f1af5a204fa3a358012 Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Mon, 17 Aug 2026 16:19:48 -0400 Subject: [PATCH 4/5] Route Firefox :: IP Protection to #team-eng-ip-protection-triage, and tell the agent which of its two state machines it is looking at 22 of the 32 open defects filed in `Firefox :: IP Protection` over the last 90 days came from staff, second only to New Tab Page's 90, so this is the highest-volume component added in a while. `TRIAGE_SCOPE` gets one entry. Both of its directories already sit inside the `browser/` and `toolkit/` list the prompt gives, so unlike Site Permissions this needs no rescue from being read as a Core bug -- what it needs is to know that the module is split and that the symptom and the cause are usually on opposite sides. The panel is in `browser/components/ipprotection/`; the state is in `toolkit/components/ipprotection/`. There are **two** state machines, both with a `READY`, and conflating them is the mistake worth pre-empting: `IPProtectionStates` in `IPProtectionService.sys.mjs` is entitlement and sign-in (`UNINITIALIZED`, `UNAVAILABLE`, `UNAUTHENTICATED`, `READY`, firing `IPProtectionService:StateChanged`), while `IPPProxyStates` in `IPPProxyManager.sys.mjs` is the connection (`NOT_READY`, `READY`, `ACTIVATING`, `ACTIVE`, `ERROR`, `PAUSED`, firing `IPPProxyManager:StateChanged`). "It showed connected when it was not" is the second and lives in `toolkit/`, even though every visible trace of it is in the panel -- so an agent that starts where the screenshot points lands in the wrong tree. The bullet also sends the agent to `toolkit/components/ipprotection/docs/`, which has `StateMachine.rst`, `Preferences.rst`, `Constants.rst` and `Components.rst`. No other area here has in-tree prose docs, and reading them beats reconstructing the machine from source. Two corrections it needs on top of the generic advice: prefs are `browser.ipProtection.*` in `browser/app/profile/firefox.js`, not `modules/libpref/init/all.js` where the prompt's generic pref line points (25 hits against 0), and a `browser/` to `toolkit/` split is still in flight, so Searchfox is more trustworthy than the shallow checkout for a path. `severity-assessment.md` gets a paragraph, in the same shape as the install-and-update one. Turning the VPN off is not a workaround for the VPN not working; it is the absence of the thing the user is paying for. So start from S2 rather than the S3 a papercut gets. The paragraph then asks for a distinction the reports will not make on their own: state merely *displayed* wrong is a UI bug, state actually wrong means traffic is unproxied and belongs above S2. Both arrive as "it said I was protected". The README's area list now matches the registry's six areas. It had four, having grouped the updater and the installer as "install and update" -- which reads fine as prose but left a reader unable to tell that IP Protection is triaged at all, and the list is now the only place the README names what is in scope. `TRIAGE_TASK` in `__main__.py` already reads "user-facing Firefox bug" after ecea6ca6, so it needs no widening; the `description` in services/hackbot-api is console UI text that never reaches the model, and IP Protection is desktop frontend for its purposes. Neither is touched. 46 tests pass, with no test edit for the routing -- f076cc9e derives it. The new `IP Protection` area is what `test_every_area_has_prompt_guidance` was for: it failed until the prompt bullet existed. **This must be deployed before bugbot starts sending**, and before that someone has to confirm `#team-eng-ip-protection-triage` is the exact name and is public. `channel_for` fails closed, so a wrong name means the comment and the severity change still land with nobody told, and a private channel the app was not invited to fails the same way -- `chat:write.public` only covers public ones. --- agents/frontend-triage/README.md | 36 +++++++++++++------ .../hackbot_agents/frontend_triage/config.py | 6 ++++ .../frontend_triage/prompts/system.md | 6 ++++ .../rules/severity-assessment.md | 10 ++++++ 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/agents/frontend-triage/README.md b/agents/frontend-triage/README.md index 514a35b056..fe816b1644 100644 --- a/agents/frontend-triage/README.md +++ b/agents/frontend-triage/README.md @@ -14,19 +14,26 @@ human (or a downstream execution agent) takes it from there. ## What it triages **Defects in user-facing Firefox** — the kind documented with a screenshot, steps -to reproduce, or a log rather than a stack trace, across four areas: +to reproduce, or a log rather than a stack trace. `scoping.md` is what decides +scope, and it is broad: any user-facing Firefox defect qualifies. + +What is _routed_ is narrower. `TRIAGE_SCOPE` in `config.py` lists the components +bugs normally arrive from, one entry each, carrying the Slack channel and the area +whose code layout `prompts/system.md` describes. A bug handed to the agent by hand +in some other component — `Firefox :: Menus`, say — is triaged the same way and +reports to nobody. The areas, which are also how the rendered scope list is +grouped: - **Desktop frontend**, under `Firefox`. JS/JSM modules, CSS, XUL/HTML. - **Site permissions**, also desktop, but split across the doorhanger, the state, and a C++ store outside `browser/`. +- **IP Protection**, the built-in VPN. Panel UI in + `browser/components/ipprotection/`, the proxy and entitlement state machines in + `toolkit/components/ipprotection/`. - **Firefox for Android**. Kotlin under `mobile/android/fenix/` and `mobile/android/android-components/`. -- **Install and update**: `Firefox :: Installer` (NSIS) and - `Toolkit :: Application Update` (`.sys.mjs`, IDL, C++). - -`TRIAGE_SCOPE` in `config.py` is the component list, one entry per component, -carrying the area and the Slack channel. `prompts/system.md` renders it into the -system prompt and describes each area's code layout. +- **Application updater** — `Toolkit :: Application Update` (`.sys.mjs`, IDL, C++). +- **Windows installer** — `Firefox :: Installer` (NSIS). Install and update bugs are the odd ones out: they arrive as a failure with an error code and an `update.log` or installer log, usually with no steps to @@ -36,6 +43,11 @@ framing reads as a reason to skip them. `severity-assessment.md` starts them at S2 rather than the S3 a papercut would get, since a user who cannot update is left on an unpatched build with no in-product workaround. +IP Protection has the same S2 floor, for the same reason: turning the VPN off is +not a workaround for it not working. It carries one extra instruction, because the +distinction does not survive a bug report — state merely _displayed_ wrong is a UI +bug, while state actually wrong means traffic is unproxied and belongs above S2. + Poor fits: crashes, hangs, assertions and sanitizer reports (those belong to [`bug-fix`](../bug-fix/)) — note that "the installer failed" is not a crash report — anything outside user-facing Firefox, and bugs whose fix can only be @@ -187,12 +199,14 @@ Four things about that which are not obvious from reading the registry: - **The key is the component, not the team**, so two components may share a channel, as the installer and the updater do, without either knowing about the other. -- **A `channel=None` entry is in scope and notifies nobody.** The agent has a ruleset - for it if a human hands it that bug; no team is told. This is not the same as being - outside bugbot's automatic scope — bugbot decides what gets sent, and a pair it sends - whose component has no channel here gets unattended triage in silence. - **There is deliberately no default channel**, since posting one team's triage into another team's channel is worse than silence. +- **`TRIAGE_SCOPE` is narrower than what the agent will triage.** It is the routing + table, and it should stay in step with bugbot's `TRIAGED_COMPONENTS`, which decides + what arrives automatically. `scoping.md` puts _any_ user-facing Firefox defect in + scope, so a bug handed to the agent by hand in some other component is triaged + normally and reports to nobody. The system prompt says so explicitly, because a list + of components read as exhaustive is how an in-scope bug gets declared out of scope. - **Product and component come from the agent's `product`/`component` plan fields**, because nothing else carries them out of a run whose only input is a bug id. A garbled value matches no team and sends nothing, which is why the system prompt asks for them diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py index 523493c1a4..be41c6bd29 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py @@ -91,6 +91,12 @@ def key(self) -> str: ScopedComponent( "Firefox", "Site Permissions", "Site permissions", "#privacy-team-automation" ), + ScopedComponent( + "Firefox", + "IP Protection", + "IP Protection", + "#team-eng-ip-protection-triage", + ), ScopedComponent( "Firefox for Android", "History", "Firefox for Android", "#android-core-dev" ), 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 62f1779adf..8d8bd8b65d 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md @@ -39,6 +39,11 @@ Where to look, and what you will find there, depends on the bug's component: - **Desktop frontend** — `browser/`, `toolkit/`, and `devtools/`. JS/JSM modules (`.js`, `.mjs`, `.sys.mjs`), CSS, and XUL/HTML. - **Site permissions** — desktop JS, but split across the prompt, the state, and the store, so start by working out which of the three the bug is in. `browser/modules/SitePermissions.sys.mjs` holds the permission state the rest of the frontend reads and writes, including the defaults, the scopes (`SCOPE_PERSISTENT`, `SCOPE_SESSION`, `SCOPE_TEMPORARY`), and the `ALLOW`/`BLOCK`/`PROMPT` states. `browser/modules/PermissionUI.sys.mjs` builds the doorhanger prompts, one subclass per permission type. `browser/actors/WebRTCParent.sys.mjs` handles camera, microphone, and screen sharing, which do **not** go through the generic prompt path and carry their own sharing indicator. The management UI is `browser/components/preferences/dialogs/permissions.js` and `sitePermissions.js`. The backing store is `nsIPermissionManager`, implemented in C++ at `extensions/permissions/PermissionManager.cpp` — that is outside the frontend directories, so "the permission did not stick", "it came back after a restart", and wrong-expiry bugs are localized there and are **not** out of scope for being non-JS. +- **IP Protection** — the built-in VPN, desktop JS in two trees, and which tree matters more than which file. `browser/components/ipprotection/` is the UI and the per-window glue: `IPProtection.sys.mjs` (`EveryWindow` and `CustomizableUI` registration), `IPProtectionPanel.sys.mjs` (panel lifecycle and the only sanctioned way to change what the panel shows, `setState`), `IPProtectionToolbarButton.sys.mjs`, `IPProtectionInfobarManager.sys.mjs`, `IPProtectionAlertManager.sys.mjs`, and one-concern `IPP*Helper.sys.mjs` files for onboarding, opt-out, and usage. The panel's own markup is Lit components under `content/*.mjs` (`ipprotection-content.mjs`, `ipprotection-status-card.mjs`, `ipprotection-locations.mjs`, `ipprotection-message-bar.mjs`), with shared values — thresholds, URLs, country-to-flag maps — in `content/ipprotection-constants.mjs`. `toolkit/components/ipprotection/` is the platform-agnostic service layer: `IPProtectionService.sys.mjs`, `IPPProxyManager.sys.mjs`, `IPPChannelFilter.sys.mjs` (which traffic is proxied), `IPPNetworkErrorObserver.sys.mjs`, `IPProtectionServerlist.sys.mjs`, `IPPAuthProvider.sys.mjs`, `IPPExceptionsManager.sys.mjs` (per-site exclusions), `IPPNimbusHelper.sys.mjs`. + - **State lives in the service, not the panel**, so a bug whose symptom is in the panel usually is not. There are **two** state machines and both have a `READY`: `IPProtectionStates` in `IPProtectionService.sys.mjs` is entitlement and sign-in (`UNINITIALIZED`, `UNAVAILABLE`, `UNAUTHENTICATED`, `READY`) and fires `IPProtectionService:StateChanged`; `IPPProxyStates` in `IPPProxyManager.sys.mjs` is the connection (`NOT_READY`, `READY`, `ACTIVATING`, `ACTIVE`, `ERROR`, `PAUSED`) and fires `IPPProxyManager:StateChanged`. Say which one you mean. "It showed connected when it was not" and "it came back on after I turned it off" are proxy-state bugs in `toolkit/`; "the panel offered it to a user who is not entitled" is a service-state bug. The panel only reacts, through `setState`, and content components emit `IPProtection:*` events upward rather than acting. + - `toolkit/components/ipprotection/docs/` has `StateMachine.rst`, `Preferences.rst`, `Constants.rst` and `Components.rst` — in-tree prose documentation, which none of the other areas here has. **Read it before reasoning about a state transition**; it is faster and more reliable than reconstructing the machine from the source. + - A `browser/` → `toolkit/` split is in progress, so both trees can hold a plausible-looking copy of the same concern and the shallow local checkout may be behind. Prefer `search_identifier` / `find_definition`, which see the indexed revision, before citing a path. + - Prefs are `browser.ipProtection.*`, registered in `browser/app/profile/firefox.js` — **not** `modules/libpref/init/all.js`. Strings are `browser/locales/en-US/browser/ipProtection.ftl`, and Glean metrics are in a `metrics.yaml` in each of the two directories. - **Firefox for Android** — `mobile/android/`, with the Fenix app under `mobile/android/fenix/app/src/main/java/org/mozilla/fenix/` and the reusable components under `mobile/android/android-components/`. This is **Kotlin**, and it is structured as Fragment / Store / Middleware / View rather than as chrome markup plus a script: a `…Fragment.kt` owns the screen, a `…FragmentStore.kt` holds its state and actions, a `…View.kt` or a Compose function renders it, and a `…Middleware.kt` performs side effects. Layouts are Android XML under `mobile/android/fenix/app/src/main/res/layout/`, strings under `res/values/strings.xml`. Fenix is mid-migration to Jetpack Compose, so a screen may have both a `…View.kt` and a `…Composable.kt` and only one of them is live — check which the Fragment actually builds before planning against either. - **Android toolbar** — there are **two** toolbars, and a generation of the widget under each. The browser toolbar is `…/fenix/components/toolbar/` (`BrowserToolbarComposable.kt`, `BrowserToolbarMiddleware.kt`, `BrowserNavigationBar.kt`, `ToolbarPosition.kt` for top-versus-bottom, `BottomToolbarContainerView.kt`, `ToolbarsIntegration.kt`); the homepage has its own at `…/fenix/home/toolbar/` (`HomeToolbarComposable.kt`, `FenixHomeToolbar.kt`, `BrowserSimpleToolbar.kt`). So work out which surface the reporter was on first: a `Homepage` bug can localize into a toolbar file and a `Toolbar` bug into the homepage. Underneath both, android-components has the newer Compose widget at `mobile/android/android-components/components/compose/browser-toolbar/` and the older View-based one at `components/browser/toolbar/`, with `components/concept/toolbar/` holding the interface and `components/feature/toolbar/` the session wiring. Confirm which one Fenix builds before citing it — a fix planned against the retired implementation reads correct and changes nothing. - **Android homepage** — one screen assembled from one package per section, so "which section" comes before "which file". `…/fenix/home/HomeFragment.kt` owns the screen, the Compose UI is under `home/ui/` (`Homepage.kt`, `HomepageHeader.kt`, `SearchBar.kt`, `WallpaperBackground.kt`, `Wordmark.kt`), state is `home/store/HomepageState.kt`, side effects are `home/middleware/`, and the older controller/interactor pair is `home/sessioncontrol/`. Each section is its own subpackage: `topsites/`, `recenttabs/`, `recentsyncedtabs/`, `recentvisits/`, `pocket/`, `bookmarks/`, `collections/`, `setup/`, `sports/`, `mars/`, `logo/`, `privatebrowsing/`. A bug about the top-sites row or the stories feed is localized there, not in `Homepage.kt`. Note also that `Firefox for Android` has separate components for several of these sections — `Top Sites`, `Stories`, `Collections`, `Bookmarks`, `Menu`, `Search` — so the same code can be reached from more than one component, and `Stories` is `home/pocket/` in the tree because nothing was renamed. Triage the bug under the component it was filed in; do not retitle or re-scope it to match. @@ -48,6 +53,7 @@ Where to look, and what you will find there, depends on the bug's component: **Always look for an existing test that exercises the affected area**, and record what you find in the `relevant_tests` field — it is the downstream executor's verification anchor. Where to look depends on the component: - Desktop: browser-chrome mochitests usually live in a component's `tests/browser/` directory; also check `tests/`/`test/` and xpcshell tests. +- IP Protection: browser-chrome under `browser/components/ipprotection/tests/browser/`, which is where most of the coverage is, with shared setup in its `head.js` (`openPanel`, `closePanel`, and the panel-state helpers) — a new test almost always belongs there rather than in a bespoke setup. Also `browser/components/ipprotection/tests/xpcshell/` and, for the service layer, `toolkit/components/ipprotection/tests/xpcshell/`. Name the one matching the layer you localized to. - Site permissions: the prompts are covered by browser-chrome under `browser/base/content/test/permissions/`, `SitePermissions.sys.mjs` itself by `browser/modules/test/browser/`, and the store by xpcshell under `extensions/permissions/test/`. Name the one that matches the layer you localized to, not whichever you found first. - Android: Kotlin unit tests under `mobile/android/fenix/app/src/test/java/org/mozilla/fenix/`, and instrumented UI tests under `app/src/androidTest/`. The test tree mirrors the source packages, so name the mirror of the package you localized to — `…/test/java/org/mozilla/fenix/components/toolbar/` for the browser toolbar, `…/fenix/home/topsites/` for a top-sites bug — rather than the screen-level `HomeFragmentTest.kt`. A Compose surface may be covered only by an `androidTest` UI test; say so rather than reporting no coverage. - Updater: `toolkit/mozapps/update/tests/` — xpcshell under `unit_aus_update/`, `unit_background_update/`, and `unit_update_binary/`, browser-chrome under `browser/`, plus `marionette/` and C++ `gtest/`. diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/rules/severity-assessment.md b/agents/frontend-triage/hackbot_agents/frontend_triage/rules/severity-assessment.md index 9ee14a3640..bf62f41956 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/rules/severity-assessment.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/rules/severity-assessment.md @@ -28,6 +28,16 @@ user is affected, how many users hit it, and whether a workaround exists. failure specific to one antivirus product or one locale is narrower than one that hits a whole channel or OS version — but start from **S2** and move up or down from there, rather than starting from S3. +- **IP Protection failures do not default to S3 either.** The user is paying for the + feature, and turning it off is not a workaround for it not working — it is the absence + of the thing they bought. A proxy that will not connect, that drops without saying so, + or that reports itself active while traffic goes around `IPPChannelFilter` leaves them + without the protection they believe they have. Start from **S2** and move up or down on + reach, rather than starting from S3. Separate two cases that read identically in a bug + report: the state is merely _displayed_ wrong (the panel disagrees with the proxy) or + it is actually wrong (traffic is unproxied). The first is a UI bug, the second is a + privacy exposure and belongs above S2. Say which one you concluded and what in the code + told you. - Weigh: is it functional vs cosmetic? Is there a workaround? How frequently and how broadly is it hit (mainline path vs rare configuration)? - Do **not downgrade** an existing higher severity unless you have strong evidence the From 2c63b408b89ec48b66e49728bf51b527465828f0 Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Mon, 17 Aug 2026 17:01:31 -0400 Subject: [PATCH 5/5] Route Firefox :: Sharing to #content-sharing-automation, and tell the agent the platform half lives in widget/ 34 of the 38 open defects filed in `Firefox :: Sharing` over the last 90 days came from staff, more than IP Protection's 22, so this is a high-volume addition. The prompt bullet is the point of this commit, because `Sharing` is the one area here whose code reaches outside the `browser/`, `toolkit/`, `devtools/` list the desktop frontend bullet gives. `browser/modules/SharingUtils.sys.mjs` populates the menu and gates on `BrowserUtils.getShareableURL`, and `browser/components/contentsharing/` holds the newer remotely-configured piece, validated against `contentsharing.schema.json`. But the platform half is `widget/nsIMacSharingService.idl` with `widget/cocoa/nsMacSharingService.mm`, and `shareUrl` on `widget/nsIWindowsUIUtils.idl`. So "the Share menu is empty", "the wrong apps are listed" and "Share does nothing" are per-OS bugs localized in Objective-C++ or C++ that the prompt would otherwise imply are out of scope -- the same trap `d3f0ed05` had to fix for the C++ permission store. The bullet also separates the two unrelated things this tree calls sharing. This component is sharing a URL out to another app; the sharing indicator, the "stop sharing" button and per-tab sharing state are WebRTC screen and camera capture in `browser/actors/WebRTCParent.sys.mjs`, which is site permissions' area. A grep for `sharing` returns both, and the WebRTC one has far more hits, so an agent that greps first lands in the wrong component. The test bullet names the `ContentSharingMockServer.sys.mjs` helper rather than leaving the agent to propose stubbing the config fetch by hand, points at the valid/invalid schema fixtures under `tests/unit/` as the cheap regression anchor for a config-parsing bug, and says outright that the `widget/` half has no automated coverage -- better than an empty `relevant_tests` the reader cannot interpret. No severity paragraph. Sharing failing has an in-product workaround: copy the link. The S3 papercut default is right, unlike the updater or the VPN. Two files, which is what a component in a new area costs now: the registry entry plus the `Source repository` bullet that `test_every_area_has_prompt_guidance` demands. 46 tests pass. Deploy before the matching bugbot change, and confirm `#content-sharing-automation` exists and is public first -- `channel_for` fails closed, so a wrong name still lets the comment and severity change land with nobody told. --- .../frontend-triage/hackbot_agents/frontend_triage/config.py | 1 + .../hackbot_agents/frontend_triage/prompts/system.md | 3 +++ 2 files changed, 4 insertions(+) diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py index be41c6bd29..c01bc06f05 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py @@ -91,6 +91,7 @@ def key(self) -> str: ScopedComponent( "Firefox", "Site Permissions", "Site permissions", "#privacy-team-automation" ), + ScopedComponent("Firefox", "Sharing", "Sharing", "#content-sharing-automation"), ScopedComponent( "Firefox", "IP Protection", 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 8d8bd8b65d..4971b288d4 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md @@ -39,6 +39,8 @@ Where to look, and what you will find there, depends on the bug's component: - **Desktop frontend** — `browser/`, `toolkit/`, and `devtools/`. JS/JSM modules (`.js`, `.mjs`, `.sys.mjs`), CSS, and XUL/HTML. - **Site permissions** — desktop JS, but split across the prompt, the state, and the store, so start by working out which of the three the bug is in. `browser/modules/SitePermissions.sys.mjs` holds the permission state the rest of the frontend reads and writes, including the defaults, the scopes (`SCOPE_PERSISTENT`, `SCOPE_SESSION`, `SCOPE_TEMPORARY`), and the `ALLOW`/`BLOCK`/`PROMPT` states. `browser/modules/PermissionUI.sys.mjs` builds the doorhanger prompts, one subclass per permission type. `browser/actors/WebRTCParent.sys.mjs` handles camera, microphone, and screen sharing, which do **not** go through the generic prompt path and carry their own sharing indicator. The management UI is `browser/components/preferences/dialogs/permissions.js` and `sitePermissions.js`. The backing store is `nsIPermissionManager`, implemented in C++ at `extensions/permissions/PermissionManager.cpp` — that is outside the frontend directories, so "the permission did not stick", "it came back after a restart", and wrong-expiry bugs are localized there and are **not** out of scope for being non-JS. +- **Sharing** — sending the current page to another app, and the one area here whose code reaches outside `browser/`, `toolkit/` and `devtools/`. `browser/modules/SharingUtils.sys.mjs` is the frontend: it populates the share menu, gates on `BrowserUtils.getShareableURL` (which is why an unshareable scheme silently yields no menu item), and then hands off to the platform. `browser/components/contentsharing/` is the newer piece — `ContentSharingUtils.sys.mjs`, the remotely-delivered config validated against `contentsharing.schema.json`, `content/`, and its own `metrics.yaml`. **The platform half is in `widget/`**, which the desktop-frontend bullet above does not cover: `widget/nsIMacSharingService.idl` with `widget/cocoa/nsMacSharingService.mm` (Objective-C++ — the macOS share sheet, `getSharingProviders`, `openSharingPreferences`), and `widget/nsIWindowsUIUtils.idl`'s `shareUrl` for Windows. So "the Share menu is empty", "the wrong apps are listed", and "Share does nothing" are usually localized in `widget/`, per-OS, and are **not** out of scope for being C++ rather than JS. Note which OS the bug is about before reading either. + - **Two unrelated things are called "sharing" in this tree.** This component is sharing a URL _out_ to another app. Screen, camera and microphone sharing — the sharing indicator, "stop sharing" button, and per-tab sharing state — is WebRTC, lives in `browser/actors/WebRTCParent.sys.mjs`, and belongs to site permissions. A grep for `sharing` returns both, so check which one the report is actually about; a bug about an indicator or a "stop sharing" control is almost certainly the WebRTC one. - **IP Protection** — the built-in VPN, desktop JS in two trees, and which tree matters more than which file. `browser/components/ipprotection/` is the UI and the per-window glue: `IPProtection.sys.mjs` (`EveryWindow` and `CustomizableUI` registration), `IPProtectionPanel.sys.mjs` (panel lifecycle and the only sanctioned way to change what the panel shows, `setState`), `IPProtectionToolbarButton.sys.mjs`, `IPProtectionInfobarManager.sys.mjs`, `IPProtectionAlertManager.sys.mjs`, and one-concern `IPP*Helper.sys.mjs` files for onboarding, opt-out, and usage. The panel's own markup is Lit components under `content/*.mjs` (`ipprotection-content.mjs`, `ipprotection-status-card.mjs`, `ipprotection-locations.mjs`, `ipprotection-message-bar.mjs`), with shared values — thresholds, URLs, country-to-flag maps — in `content/ipprotection-constants.mjs`. `toolkit/components/ipprotection/` is the platform-agnostic service layer: `IPProtectionService.sys.mjs`, `IPPProxyManager.sys.mjs`, `IPPChannelFilter.sys.mjs` (which traffic is proxied), `IPPNetworkErrorObserver.sys.mjs`, `IPProtectionServerlist.sys.mjs`, `IPPAuthProvider.sys.mjs`, `IPPExceptionsManager.sys.mjs` (per-site exclusions), `IPPNimbusHelper.sys.mjs`. - **State lives in the service, not the panel**, so a bug whose symptom is in the panel usually is not. There are **two** state machines and both have a `READY`: `IPProtectionStates` in `IPProtectionService.sys.mjs` is entitlement and sign-in (`UNINITIALIZED`, `UNAVAILABLE`, `UNAUTHENTICATED`, `READY`) and fires `IPProtectionService:StateChanged`; `IPPProxyStates` in `IPPProxyManager.sys.mjs` is the connection (`NOT_READY`, `READY`, `ACTIVATING`, `ACTIVE`, `ERROR`, `PAUSED`) and fires `IPPProxyManager:StateChanged`. Say which one you mean. "It showed connected when it was not" and "it came back on after I turned it off" are proxy-state bugs in `toolkit/`; "the panel offered it to a user who is not entitled" is a service-state bug. The panel only reacts, through `setState`, and content components emit `IPProtection:*` events upward rather than acting. - `toolkit/components/ipprotection/docs/` has `StateMachine.rst`, `Preferences.rst`, `Constants.rst` and `Components.rst` — in-tree prose documentation, which none of the other areas here has. **Read it before reasoning about a state transition**; it is faster and more reliable than reconstructing the machine from the source. @@ -53,6 +55,7 @@ Where to look, and what you will find there, depends on the bug's component: **Always look for an existing test that exercises the affected area**, and record what you find in the `relevant_tests` field — it is the downstream executor's verification anchor. Where to look depends on the component: - Desktop: browser-chrome mochitests usually live in a component's `tests/browser/` directory; also check `tests/`/`test/` and xpcshell tests. +- Sharing: browser-chrome under `browser/components/contentsharing/tests/browser/`, which has a `ContentSharingMockServer.sys.mjs` for the remote config — use it rather than stubbing the fetch yourself. Schema fixtures are xpcshell under `tests/unit/` (`validContentSharing.*.json` / `invalidContentSharing.*.json`), so a config-parsing bug has a very cheap regression test. The `widget/` half is effectively uncovered: there is no automated test for the macOS share sheet or the Windows share dialog, so for a platform-side bug say the area is untested rather than leaving the reader wondering. - IP Protection: browser-chrome under `browser/components/ipprotection/tests/browser/`, which is where most of the coverage is, with shared setup in its `head.js` (`openPanel`, `closePanel`, and the panel-state helpers) — a new test almost always belongs there rather than in a bespoke setup. Also `browser/components/ipprotection/tests/xpcshell/` and, for the service layer, `toolkit/components/ipprotection/tests/xpcshell/`. Name the one matching the layer you localized to. - Site permissions: the prompts are covered by browser-chrome under `browser/base/content/test/permissions/`, `SitePermissions.sys.mjs` itself by `browser/modules/test/browser/`, and the store by xpcshell under `extensions/permissions/test/`. Name the one that matches the layer you localized to, not whichever you found first. - Android: Kotlin unit tests under `mobile/android/fenix/app/src/test/java/org/mozilla/fenix/`, and instrumented UI tests under `app/src/androidTest/`. The test tree mirrors the source packages, so name the mirror of the package you localized to — `…/test/java/org/mozilla/fenix/components/toolbar/` for the browser toolbar, `…/fenix/home/topsites/` for a top-sites bug — rather than the screen-level `HomeFragmentTest.kt`. A Compose surface may be covered only by an `androidTest` UI test; say so rather than reporting no coverage.