Conversation
An uplift run is an ordered list of sources applied onto a stable branch, so a stack is simply several sources. `kind` discriminates the two shapes a source can take, letting one run mix them. Each kind knows two things about itself: how to materialize whatever the agent needs on disk (`fetch_diff`) and how to describe itself as a work item in the prompt (`render_work_item`). Keeping both on the model means a third source kind touches this module only. A Phabricator source carries only `revision_id` plus an optional `diff_id`, not the diff text. Run inputs reach the Cloud Run Job as environment variables, so an inlined diff would be bounded by the env size limit. `diff_id` defaults to the revision's latest, but a caller that knows which diff it landed should pin it: otherwise a revision updated between request and run resolves to different code. Diffs are named on disk by diff as well as revision, since sources are all fetched up front and two pins on one revision would otherwise share a file. A fetched diff carries its author and its base commit. The author because the uplift commit must keep it -- Lando refuses a patch authored by hackbot -- and the base commit because `git apply --3way` needs it fetched to find the blobs the diff names. moz-phab abbreviates that hash for a repo the size of firefox and `git fetch` refuses an abbreviation, so it is expanded before use. `RequestedSource` is the record of what a run was asked to uplift and what the fetch pinned each source down to, which for an unpinned source is not derivable from the input. It is not evidence anything was applied. `Report` is the schema of the `report.json` the agent writes, which is model output and so validated rather than trusted. Every field defaults, so a partial report still parses. What validation buys is catching a plausible but wrong value: `"resolved": "false"` is a string `bool()` reads as `True`, and `confidence` is what a caller gates on. Issue: mozilla#6865
Split the way every other agent splits them: `system.md` is how to resolve an uplift and holds no per-run detail, `task.md` is what this run is uplifting, onto what, and where to report it. `system.md` covers how to apply each kind of source, how to resolve a conflict without dropping the patch's functional change, which Mozilla MCP to consult when the intent is unclear, and the report shape. Four things it insists on: confidence is graded conservatively, since a wrong uplift on a stable branch is expensive and the grade is what a human triages on; every commit keeps the original patch author, because Lando refuses a patch authored by hackbot; a Phabricator diff's base commit is fetched before applying it, without which the three-way merge finds no blobs and leaves nothing to resolve; and the checkout has to agree with the report, since that is checked mechanically. Keeping the run out of `system.md` leaves an identical prefix on every run for prompt caching to reuse, and keeps Phabricator-supplied strings -- an author name reaches the prompt through `--author` -- out of the most privileged part of the context. Issue: mozilla#6865
The agent grades its own work, and a caller gates landing on `resolved`. Taken on trust that accepts a report claiming success while listing unresolved hunks, a tree with conflict markers still in the index, a cherry-pick left open, work left uncommitted, or a claimed resolution with no patch behind it at all. None of those are judgment calls, so they are checked against the repository. Confidence stays the agent's own grade, which only it can give. "There is a patch" is checked the way the runtime collects one, since HEAD merely differing from the commit the run started at does not mean there is something to collect: HEAD can move backwards, and an empty commit moves it while changing nothing. So the base has to remain an ancestor, the range has to hold commits, and those commits have to change something. Tested against real repositories, including a genuinely conflicted cherry-pick: these are statements about git's own state, and a stub could only assert we asked for it. The fixtures neutralise global and system git config, since a developer's commit signing or hooks path would otherwise reach these repositories -- and the runtime's own git calls too. Issue: mozilla#6865
A single-stage claude-agent-sdk run: ask each source to materialize what it needs on disk, render the task against the result, drive the session, then check the checkout and report what happened. The runtime collects the resolved commits separately into `changes/changes.patch`. The per-kind differences live on the source models, so this module only zips sources against their fetch results and lets each render itself. Diffs come from the broker's read-only Conduit proxy rather than arriving as input. The result records `base_commit` and `requested_sources`, so a reviewer can see what the run was applied onto and what each source resolved to without re-deriving it. A `resolved` the checks disagree with is overruled to `false`, with the reasons in `verification_failures`. `report.json` is published with that verified outcome rather than the agent's claim, which would otherwise contradict the run summary; what the agent wrote is published verbatim beside it as `report.unverified.json`. No Firefox build tools: resolving a conflict is a source-level judgment, and a build would need the `mach bootstrap` toolchain `build-repair` carries. `setting_sources` loads `project` so the agent reads the checkout's own `CLAUDE.md` and in-tree skills; `local` names a gitignored file a fresh clone cannot have. `effort` is omitted unless a run supplies one, since the API already defaults to `high`. `run_session` is a seam, so `run_uplift` is driven end to end against a real checkout the stand-in session commits into -- including once all the way through, resolving a genuine conflict and reapplying the collected patch to a clean clone, since a patch that will not land is the one way the run can look successful and be useless. Issue: mozilla#6865
`AgentInputs` reads the per-run inputs the platform passes as environment
variables, then `main` prepares the checkout and hands off to `run_uplift`.
The checkout is pinned before anything else reads the tree, to `target_commit`
when the caller supplied one and the branch tip otherwise: a branch name moves,
so pinning is what reproduces the checkout a failed uplift was seen on. Empty
strings count as absent, since compose passes every unset optional input as
`${VAR:-}`.
Issue: mozilla#6865
The agent container is the least-trusted component in the system and binds no durable credential, so the Phabricator key lives in a sidecar that serves `phabricator_proxy`'s read-only Conduit mount over loopback and substitutes the real key. The agent sends a placeholder token it never has to hold. Only `/phabricator/api` is mounted, unlike `bug-fix`'s broker: this agent's model-facing Bugzilla and Phabricator tools come from the bugbug MCP, and the broker exists solely so agent code can fetch each source's diff before the prompt is rendered. The methods that needs are already on the proxy's allow list. The security docs gain that Conduit proxy mount, a third shape of broker capability they did not describe, alongside the agents running a broker. Issue: mozilla#6865
One image with two targets, following `bug-fix`: `agent` runs the resolver, `broker` the Conduit proxy. `hackbot.toml` asks the platform for a Firefox checkout, the one capability the agent needs prepared -- no `[firefox]` table, since it never builds, and so no ffmpeg either. Compose runs the agent against the bugbug MCP and the broker so a local run takes the same path as a deployed one, minus the uploader. Both sidecars hold the credentials; the agent service binds none. Issue: mozilla#6865
`UpliftInputs` is the agent's public contract; env vars derive from it, so no `build_env` is needed. `target_commit` is optional, so Lando can pin the checkout it failed on once it knows to. The registry entry leaves `auto_apply_actions` off: the agent records no actions, and its output is a patch for a human to review. The UI gets the agent in its shared list, which backs both the trigger form and the run filter, plus the fields a run actually needs -- without them the form falls through to its bug-id-only default and submits without `target_branch` or `sources`. `parseUpliftSources` validates the stack against the same shapes `UpliftSource` accepts, so a malformed entry is a message beside the field rather than a 422 after submitting. Issue: mozilla#6865
The inputs, the outputs, and how to run the agent locally. Notes that `report.json` is the verified report and `report.unverified.json` is the agent's own, since `resolved` differs between them exactly when the checks rejected a claim. `agents/uplift/` is framed as a home for several uplift sub-agents, since the merge-conflict resolver is the first of them. Issue: mozilla#6865
There was a problem hiding this comment.
🟡 Changes recommended
Report validation, patch verification, commit metadata preservation, and pinned local execution have correctness gaps.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an uplift agent that resolves Firefox backport conflicts and publishes verified patches and reports for human review.
Changes:
- Adds uplift-agent orchestration, prompting, verification, broker, packaging, and deployment configuration.
- Exposes uplift runs through the API and UI.
- Adds comprehensive unit and patch round-trip tests.
File summaries
| File | Description |
|---|---|
uv.lock |
Registers uplift workspace dependencies. |
services/hackbot-ui/lib/uplift.ts |
Parses uplift sources. |
services/hackbot-ui/lib/uplift.test.ts |
Tests source parsing. |
services/hackbot-ui/lib/agents.ts |
Registers the UI agent. |
services/hackbot-ui/components/TriggerForm.tsx |
Adds uplift inputs. |
services/hackbot-api/tests/test_agents.py |
Tests API registration and serialization. |
services/hackbot-api/app/schemas.py |
Defines uplift input schemas. |
services/hackbot-api/app/agents.py |
Registers the Cloud Run job. |
docs/hackbot/security.md |
Documents broker security. |
docs/hackbot/agents.md |
Documents the new agent. |
docker-compose.yml |
Includes uplift services. |
agents/uplift/tests/test_uplift_verify.py |
Tests repository verification. |
agents/uplift/tests/test_uplift_run.py |
Tests orchestration. |
agents/uplift/tests/test_uplift_report.py |
Tests report handling. |
agents/uplift/tests/test_uplift_prompt.py |
Tests prompt rendering. |
agents/uplift/tests/test_uplift_patch_roundtrip.py |
Tests patch reapplication. |
agents/uplift/tests/test_uplift_options.py |
Tests SDK options. |
agents/uplift/tests/test_uplift_main.py |
Tests entrypoint handoff. |
agents/uplift/tests/test_uplift_diffs.py |
Tests Phabricator fetching. |
agents/uplift/tests/conftest.py |
Provides Git test fixtures. |
agents/uplift/README.md |
Documents usage and outputs. |
agents/uplift/pyproject.toml |
Defines the package. |
agents/uplift/hackbot.toml |
Configures the Firefox checkout. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/verify.py |
Verifies completed resolutions. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/prompts/task.md |
Defines per-run tasks. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/prompts/system.md |
Defines resolution workflow. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/config.py |
Defines source and report models. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/broker.py |
Hosts the Conduit proxy. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/agent.py |
Implements orchestration and reporting. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/__main__.py |
Connects runtime inputs to the agent. |
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/__init__.py |
Initializes the package. |
agents/uplift/Dockerfile |
Builds agent and broker images. |
agents/uplift/compose.yml |
Configures local services. |
Review details
Suppressed comments (1)
agents/uplift/hackbot_agents/uplift_merge_conflict_resolver/config.py:237
confidenceremains optional even whenresolvedis true, so a report containing only{"resolved": true}can pass all checkout checks and publish a resolved patch with no confidence level. Since confidence is part of the resolver's review contract, add model validation that requires it for a claimed resolution.
confidence: Literal["high", "medium", "low"] | None = None
- Files reviewed: 31/33 changed files
- Comments generated: 7
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - TARGET_BRANCH=${TARGET_BRANCH:-} | ||
| - SOURCES=${SOURCES:-} |
|
|
||
| model_config = ConfigDict(extra="ignore") | ||
|
|
||
| resolved: bool = False |
| # Uncommitted work is swept into a container-authored commit, losing the | ||
| # original author. | ||
| dirty = git_output(repo, "status", "--porcelain") | ||
| if dirty: | ||
| problems.append(f"{len(dirty.splitlines())} path(s) left uncommitted") |
| elif not git_output(repo, "rev-list", f"{base_commit}..HEAD"): | ||
| problems.append("report claims resolved but no commits were produced") | ||
| elif not git_output(repo, "diff", "--name-only", base_commit, "HEAD"): | ||
| problems.append("report claims resolved but the commits change nothing") |
| kind: Literal["git"] = "git" | ||
|
|
||
| # Full git commit SHA the agent fetches and cherry-picks onto the branch. | ||
| commit: str = Field(description="Full git commit SHA to cherry-pick.") |
| leaving you no conflict markers at all. | ||
| 3. Apply it with a three-way merge so conflicts surface as markers: | ||
| `git apply --3way <diff-file>` | ||
| 4. Resolve any conflicts and commit the fixed patch. |
| <textarea | ||
| id="upliftSources" | ||
| placeholder={ | ||
| '[{"kind": "git", "commit": "abc123"},\n' + |
There was a problem hiding this comment.
This file doesn't look like a config to me :) I would expect just some constants here, maybe within a class.
There was a problem hiding this comment.
why do we need a system prompt at all?
|
|
||
| ## When you are done | ||
|
|
||
| Leave all resolved patches committed in the working tree (the platform collects |
There was a problem hiding this comment.
Per our latest discussion with @suhaibmujahid, it shouldn't commit anything (we don't even set git identity). The edits will be collected automatically in changes.patch (see build repair agent). Maybe it's different in other agents, not sure.
| @@ -0,0 +1,67 @@ | |||
| # Uplift Agents | |||
|
|
|||
| Home for the backport/uplift sub-agents that help land patches on Firefox's | |||
There was a problem hiding this comment.
Technically, this is not a sub-agent. I'm not even sure we can change the name from hackbot_agents to uplift; I thought it was just a convention. See agents/README.md
|
|
||
| For a **Git** source (a commit SHA): | ||
|
|
||
| 1. Fetch the commit and its parent so cherry-pick can three-way merge: |
There was a problem hiding this comment.
This is probably what you need here, but we prefetch 500 commits for the test-repair agent, for example. So if you're sure that the ones you need are easy to prefetch, it's another option.
| ] | ||
|
|
||
|
|
||
| class UpliftInputs(BaseModel): |
There was a problem hiding this comment.
Do you really need such a complicated input? If things can be resolved at runtime, it's better to do that, in my experience, because later you might also want to reproduce the runs locally, run evals for the agent, implement deduplication, and many of those are easier with a simple input. I ended up using just the Taskcluster task ID for CI agents, and everything else is resolved from there.
Adds the first uplift sub-agent: it reproduces a failed uplift cherry-pick on a
stable branch, resolves the merge conflicts, and returns the resulting patch
with a confidence level for review. Sources are git commits and/or Phabricator
revisions applied in order, so a stack is just several sources, and a caller can
pin both the diff to uplift and the commit to uplift onto. It never pushes or
lands anything, auto-applies no actions, and does not build Firefox — resolving
a conflict is a source-level judgment and a reviewer compiles the result.
resolvedis not taken on trust. The agent grades its own work, so after thesession the run checks the checkout — no conflicted paths, no cherry-pick left
open, nothing uncommitted, and commits that actually collect to a patch — and
overrules the claim if they disagree, publishing the verified report alongside
what the agent wrote. Confidence stays the agent's own judgment. The Lando side
of this work (the uplift checkbox, waiting on a run, retrieving the result,
Phabricator submission and human approval) is not in this PR.
Issue: #6865