diff --git a/.flake8 b/.flake8 index 6dc81cbb..fe1a357f 100644 --- a/.flake8 +++ b/.flake8 @@ -1,2 +1,3 @@ [flake8] ignore = E501,E402,E275 +extend-exclude = .venv,venv diff --git a/apis/phabricator.py b/apis/phabricator.py index a12e42d4..3b6cd05f 100644 --- a/apis/phabricator.py +++ b/apis/phabricator.py @@ -37,7 +37,7 @@ def __init__(self, config): self.url = config['url'] @logEntryExit - def submit_patches(self, bug_id, has_patches): + def submit_patches(self, bug_id, num_commits): phab_revisions = [] @retry @@ -69,19 +69,23 @@ def submit_to_phabricator(rev_id, retry_attempt=None): return phab_revision - # arc diff will squash all commits into a single commit, so we need to jump through some hoops. - # Conceptually, we are only commiting the top-most commit in the repo (and not any subsequent commits) - # If we have two commits, we'll go backwards and grab only the first commit, then go back to tip - if has_patches: - # Checkout to the first patch - self.run(["hg", "checkout", "tip^"]) - # Tell phabricator to submit from the base to the current working tree + # arc diff will squash everything from the base to the working parent into a + # single revision, so to submit each commit as its own revision we walk the + # stack from the bottom-most commit upward, submitting one commit at a time. + # (num_commits is 1 for a plain vendor, 2 when there are local patches, and 3 + # when the AI added a commit resolving patch conflicts.) + if num_commits > 1: + # Checkout to the bottom-most commit we're submitting. + self.run(["hg", "checkout", "tip~%d" % (num_commits - 1)]) + # Submit it as the diff from the base to the current working tree. phab_revisions.append(submit_to_phabricator("")) - # Ask hg to evolve the original second patch on top of the rewritten first patch - self.run(["hg", "next"]) - - # Submit only a single patch - phab_revisions.append(submit_to_phabricator("tip^")) + # Then evolve up one commit at a time, submitting each on its own. + for _ in range(num_commits - 1): + self.run(["hg", "next"]) + phab_revisions.append(submit_to_phabricator(".^")) + else: + # Submit only the single (vendoring) commit. + phab_revisions.append(submit_to_phabricator("tip^")) # Chain revisions together if needed @retry diff --git a/automation.py b/automation.py index 6f736c73..bca56f94 100755 --- a/automation.py +++ b/automation.py @@ -14,6 +14,7 @@ from components.libraryprovider import LibraryProvider from components.mach_vendor import VendorProvider from components.bugzilla import BugzillaProvider +from components.aiprovider import AIProvider from components.scmprovider import SCMProvider from components.hg import MercurialProvider, reset_repository from apis.taskcluster import TaskclusterProvider @@ -32,6 +33,7 @@ 'Taskcluster': TaskclusterProvider, 'Phabricator': PhabricatorProvider, 'SCM': SCMProvider, + 'AI': AIProvider, 'VendorTaskRunner': VendorTaskRunner, 'CommitAlertTaskRunner': CommitAlertTaskRunner } @@ -123,6 +125,7 @@ def getOr(name): 'taskclusterProvider': getOr('Taskcluster'), 'phabricatorProvider': getOr('Phabricator'), 'scmProvider': getOr('SCM'), + 'aiProvider': getOr('AI'), }) # Step 6 self.runOnProviders(lambda x: x.update_config(additional_config)) diff --git a/components/aiprovider.py b/components/aiprovider.py new file mode 100644 index 00000000..3cd68635 --- /dev/null +++ b/components/aiprovider.py @@ -0,0 +1,154 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import os +import json +import shutil +import tempfile + +from components.utilities import load_prompt +from components.logging import logEntryExit, LogLevel +from components.providerbase import BaseProvider, INeedsCommandProvider, INeedsLoggingProvider + + +CONFLICT_RESOLUTION_RESULT_FILE = "conflict_resolution_result.json" + + +class AIResult: + def __init__(self, success, text, session_id=None): + self.success = success + self.text = text + self.session_id = session_id + + +class AIProvider(BaseProvider, INeedsCommandProvider, INeedsLoggingProvider): + """ + Invokes an AI coding agent headlessly to perform a task described by a + prompt, and returns its result. Currently backed by the Claude Code CLI + (`claude -p`). + + The prompt and system prompt are written to files rather than passed on the + command line, so we never hit command-line length limits: the prompt is fed + on stdin (claude reads the prompt from stdin when given no positional + prompt) and the system prompt is passed with --append-system-prompt-file. + """ + + def __init__(self, config): + self.model = config.get('model', 'claude-opus-4-8') + self.max_turns = config.get('max-turns', 30) + self.timeout = config.get('timeout', 60 * 60) + # API key for the AI CLI, supplied through the config dictionary (like + # the Database password and Bugzilla apikey). Passed to the CLI via the + # environment variable it expects rather than on the command line. + self.apikey = config.get('apikey', None) + # For debugging: a directory into which each invocation's prompt, system + # prompt, and the CLI's raw stdout/stderr are written. None disables it. + self.debug_output_dir = config.get('debug-output-dir', None) + self._invocation_count = 0 + # A label (library + job id) folded into debug filenames, set per call. + self._debug_label = "" + + @logEntryExit + def _run_prompt(self, prompt, system_prompt=None, cwd=None): + tmpdir = tempfile.mkdtemp(prefix="updatebot-claude-") + try: + prompt_path = os.path.join(tmpdir, "prompt.txt") + with open(prompt_path, "w") as f: + f.write(prompt) + + args = ["claude", "-p", + "--output-format", "json", + "--permission-mode", "bypassPermissions", + "--model", self.model, + "--max-turns", str(self.max_turns)] + + if system_prompt: + system_path = os.path.join(tmpdir, "system_prompt.txt") + with open(system_path, "w") as f: + f.write(system_prompt) + args += ["--append-system-prompt-file", system_path] + + env = {"ANTHROPIC_API_KEY": self.apikey} if self.apikey else None + + # claude returns is_error (and a non-zero exit) on failure but still + # prints the JSON result we want to read, so don't let run() raise. + ret = self.run(args, shell=False, clean_return=False, cwd=cwd, + stdin_path=prompt_path, timeout=self.timeout, env=env) + self._write_debug_output(prompt, system_prompt, ret) + return self._parse_result(ret) + finally: + shutil.rmtree(tmpdir, ignore_errors=True) + + def _write_debug_output(self, prompt, system_prompt, ret): + # When debug-output-dir is configured, dump everything about this + # invocation so a live run can be inspected after the fact (the CLI + # buffers its JSON until completion, so nothing is available mid-run). + if not self.debug_output_dir: + return + self._invocation_count += 1 + n = self._invocation_count + try: + os.makedirs(self.debug_output_dir, exist_ok=True) + + label = ("_" + self._debug_label) if self._debug_label else "" + + def _write(suffix, contents): + path = os.path.join(self.debug_output_dir, "claude_%03d%s_%s" % (n, label, suffix)) + mode = "wb" if isinstance(contents, bytes) else "w" + with open(path, mode) as f: + f.write(contents if contents is not None else "") + + _write("prompt.txt", prompt) + if system_prompt: + _write("system_prompt.txt", system_prompt) + _write("stdout.json", ret.stdout) + _write("stderr.txt", getattr(ret, "stderr", None)) + self.logger.log("Wrote AI debug output to %s (invocation %d)" % ( + self.debug_output_dir, n), level=LogLevel.Info) + except OSError as e: + self.logger.log("Could not write AI debug output: %s" % e, level=LogLevel.Warning) + + def _parse_result(self, ret): + stdout = ret.stdout.decode() if isinstance(ret.stdout, bytes) else ret.stdout + try: + data = json.loads(stdout) + except ValueError: + self.logger.log("The AI CLI did not return parseable JSON output.", level=LogLevel.Error) + return AIResult(False, stdout) + + success = (ret.returncode == 0) and not data.get("is_error", True) + return AIResult(success, data.get("result", ""), data.get("session_id")) + + @logEntryExit + def resolve_patch_conflicts(self, moz_yaml_path, commit_message, cwd=None, + library_name=None, job_id=None): + # Ask the AI to resolve local-patch conflicts for a library. It works in + # the checkout (cwd), updates the .patch files / moz.yaml so they apply, + # and writes its verdict to CONFLICT_RESOLUTION_RESULT_FILE. Returns the + # parsed {"outcome", "details"} dict, or None if it produced no result. + # library_name/job_id only label debug output; library_name defaults to + # the moz.yaml's parent directory name. + library = library_name or os.path.basename(os.path.dirname(moz_yaml_path)) or "library" + self._debug_label = "%s_job%s" % (library, job_id if job_id is not None else "none") + + instructions = load_prompt("conflict_resolution_details", + moz_yaml_path=moz_yaml_path, + patch_fix_commit_message=commit_message) + prompt = load_prompt("conflict_resolution", conflict_resolution_instructions=instructions) + system_prompt = load_prompt("system") + + self._run_prompt(prompt, system_prompt=system_prompt, cwd=cwd) + + result_path = os.path.join(cwd, CONFLICT_RESOLUTION_RESULT_FILE) if cwd else CONFLICT_RESOLUTION_RESULT_FILE + try: + with open(result_path) as f: + resolution = json.load(f) + os.remove(result_path) + except (OSError, ValueError): + self.logger.log("Could not read %s after AI conflict resolution." % CONFLICT_RESOLUTION_RESULT_FILE, level=LogLevel.Warning) + return None + self.logger.log("AI conflict resolution outcome: %s" % resolution.get("outcome"), level=LogLevel.Info) + return resolution diff --git a/components/bugzilla.py b/components/bugzilla.py index 58d0e42a..4cabd950 100644 --- a/components/bugzilla.py +++ b/components/bugzilla.py @@ -152,6 +152,40 @@ def COULD_NOT_GENERAL_ERROR(action, errormessage=None): s += "\nUpdatebot will be unable to do anything more for this library version." return s + @staticmethod + def COULD_NOT_GENERAL_ERROR_WITH_AI(action, initialerrormessage=None, ai_outcome=None, ai_details=None): + s = "Updatebot encountered an error while trying to %s" % action + if initialerrormessage: + s += " with the following message:\n\n" + for line in initialerrormessage.split("\n"): + s += "> " + line + "\n" + s += "\nAfter getting this error, Updatebot asked its AI assistant to help" + if ai_outcome: + s += ", which reported an outcome of '%s'." % ai_outcome + else: + s += ", which seemingly failed with no outcome at all." + if ai_details: + s += "\n\n" + for line in ai_details.split("\n"): + s += "> " + line + "\n" + s += "\nUpdatebot will be unable to do anything more for this library version." + return s + + @staticmethod + def AI_RESOLVED_PATCH_CONFLICTS(outcome, details): + explanations = { + "trivial success": "This means the patches were updated in a straightforward way and should be reliable.", + "uncertain success": "This means resolving the conflicts required non-trivial judgement and the result should be reviewed carefully.", + } + s = "Updatebot's AI assistant resolved conflicts while applying the local patches and reported an outcome of '%s'.\n\n" % outcome + explanation = explanations.get(outcome) + if explanation: + s += explanation + "\n\n" + if details: + for line in details.split("\n"): + s += "> " + line + "\n" + return s + @staticmethod def COULD_NOT_VENDOR_ALL_FILES(library, errormessage): s = "`./mach vendor %s` reported an error editing moz.build files:\n" % library.yaml_path diff --git a/components/commandprovider.py b/components/commandprovider.py index 51db81cf..1aaa87e4 100644 --- a/components/commandprovider.py +++ b/components/commandprovider.py @@ -20,6 +20,7 @@ def _update_config(self, additional_config): self.infolog = partial(self.logger.log, level=LogLevel.Info) self.debuglog = partial(self.logger.log, level=LogLevel.Debug) - def run(self, args, shell=False, clean_return=True): + def run(self, args, shell=False, clean_return=True, cwd=None, stdin_path=None, timeout=60 * 20, env=None): return _run(args, shell=shell, clean_return=clean_return, - errorlog=self.errorlog, infolog=self.infolog, debuglog=self.debuglog) + errorlog=self.errorlog, infolog=self.infolog, debuglog=self.debuglog, + cwd=cwd, stdin_path=stdin_path, timeout=timeout, env=env) diff --git a/components/commandrunner.py b/components/commandrunner.py index bdcc8a9d..a2a85011 100644 --- a/components/commandrunner.py +++ b/components/commandrunner.py @@ -23,7 +23,8 @@ def do_nothing(*args, **kwargs): """ -def _run(args, shell, clean_return, errorlog=do_nothing, infolog=do_nothing, debuglog=do_nothing): +def _run(args, shell, clean_return, errorlog=do_nothing, infolog=do_nothing, debuglog=do_nothing, + cwd=None, stdin_path=None, timeout=60 * 20, env=None): ran_to_completion = False stdout = None stderr = None @@ -49,9 +50,15 @@ def _run(args, shell, clean_return, errorlog=do_nothing, infolog=do_nothing, deb start = time.time() infolog("Running", args) + # Optionally feed a file to the process's stdin (used to pass large input, + # e.g. a Claude prompt, without hitting command-line length limits). + stdin_handle = open(stdin_path, "rb") if stdin_path else None + # Merge any extra env (e.g. secrets) over the inherited environment. + run_env = {**os.environ, **env} if env else None try: ret = subprocess.run( - args, shell=shell, stdout=PIPE, stderr=PIPE, timeout=60 * 20) + args, shell=shell, stdout=PIPE, stderr=PIPE, timeout=timeout, + cwd=cwd, stdin=stdin_handle, env=run_env) except subprocess.TimeoutExpired as e: ran_to_completion = False stdout = e.stdout @@ -61,6 +68,9 @@ def _run(args, shell, clean_return, errorlog=do_nothing, infolog=do_nothing, deb ran_to_completion = True stdout = ret.stdout.decode() stderr = ret.stderr.decode() + finally: + if stdin_handle: + stdin_handle.close() if not ran_to_completion: errorlog("Command Timed Out. Will abort....") diff --git a/components/scmprovider.py b/components/scmprovider.py index acd9134e..cb27ab8d 100644 --- a/components/scmprovider.py +++ b/components/scmprovider.py @@ -30,6 +30,16 @@ def repo_and_commit_to_url(repo, commit): return repo.replace(".git", "") + "/commit/" + commit +def repo_and_compare_url(repo, old_commit, new_commit): + if not repo or not any(h in repo for h in ["github.com", "gitlab.com"]): + return None + return repo.replace(".git", "") + "/compare/" + old_commit + "..." + new_commit + + +# Separator line placed between commit blocks in generated bug comments. +COMMENT_SEPARATOR = "----------------------------------------\n" + + class Commit: def __init__(self, pretty_line): parts = pretty_line.split("|") @@ -42,6 +52,10 @@ def __init__(self, pretty_line): self.files_deleted = [] self.files_other = [] + # The parent (first-parent) revision, resolved to a concrete hash in + # populate_details. Used to build a compare URL whose range includes + # this commit. + self.parent_revision = None self.populated = False def populate_details(self, repo, run): @@ -49,6 +63,7 @@ def populate_details(self, repo, run): return rev_range = [self.revision + "^", self.revision] + self.parent_revision = run(["git", "rev-parse", self.revision + "^"]).stdout.decode().strip() files_changed = run(["git", "diff", "--name-status"] + rev_range).stdout.decode().split("\n") for f in files_changed: @@ -239,65 +254,77 @@ def _print_differing_commit_lists(self, list_a, list_a_name, list_b, list_b_name self.logger.log(" - %s" % c, level=LogLevel.Error) raise Exception(problem) - def build_bug_description(self, list_of_commits, max_length): - # The commits are ordered oldest to newest. - # But when we file a bug we want the newest commit to be at the top. - list_of_commits = copy.deepcopy(list_of_commits) + # ================================================================= + + def _commit_block(self, c, verbosity): + # Render a single commit at the given verbosity: + # 1: revision, author and the revision link + # 2: + authored/committed dates and the commit summary + # 3: + the full description and the lists of changed files + if not c.populated: + raise Exception("Tried to build bug description; but commit details not populated.") + + s = "%s by %s\n" % (c.revision, c.author) + s += c.revision_link + "\n" + + if verbosity >= 2: + s += "Authored: %s\n" % (c.author_date) + s += "Committed: %s\n" % (c.commit_date) + s += "\n" + s += c.summary + "\n" + + if verbosity >= 3: + s += "\n" + s += c.description + "\n" + + if c.files_added: + s += "\n" + s += "Files Added:\n" + for f in c.files_added: + s += " - %s\n" % f + + if c.files_deleted: + s += "\n" + s += "Files Deleted:\n" + for f in c.files_deleted: + s += " - %s\n" % f + + if c.files_modified: + s += "\n" + s += "Files Modified:\n" + for f in c.files_modified: + s += " - %s\n" % f + + if c.files_other: + s += "\n" + s += "Files Changed:\n" + for f in c.files_other: + s += " - %s\n" % f + return s + + # ----------------------------------------------------------------- + + def _build_single_comment_description(self, list_of_commits, max_length): + # Render all the commits into a single comment. + # Reducing the verbosity until the result fits within max_length. + + # The caller has already copied the list; reorder it newest-first for display. list_of_commits.reverse() def _get_details(verbosity): if verbosity == 0: - s = "----------------------------------------\n" + s = COMMENT_SEPARATOR s += "%s commits elided, as they are too long for a bugzilla comment.\n\n" % len(list_of_commits) - s += "----------------------------------------\n" + s += COMMENT_SEPARATOR return s - s = "----------------------------------------\n" + s = COMMENT_SEPARATOR for c in list_of_commits: - if not c.populated: - raise Exception("Tried to build bug description; but commit details not populated.") - - s += "%s by %s\n" % (c.revision, c.author) - s += c.revision_link + "\n" - - if verbosity >= 2: - s += "Authored: %s\n" % (c.author_date) - s += "Committed: %s\n" % (c.commit_date) - s += "\n" - s += c.summary + "\n" - - if verbosity >= 3: - s += "\n" - s += c.description + "\n" - - if c.files_added: - s += "\n" - s += "Files Added:\n" - for f in c.files_added: - s += " - %s\n" % f - - if c.files_deleted: - s += "\n" - s += "Files Deleted:\n" - for f in c.files_deleted: - s += " - %s\n" % f - - if c.files_modified: - s += "\n" - s += "Files Modified:\n" - for f in c.files_modified: - s += " - %s\n" % f - - if c.files_other: - s += "\n" - s += "Files Changed:\n" - for f in c.files_other: - s += " - %s\n" % f - s += "\n----------------------------------------\n" + s += self._commit_block(c, verbosity) + s += "\n" + COMMENT_SEPARATOR return s - # Bugzilla's limit is 65535 details = _get_details(verbosity=3) if len(details) > max_length: details = _get_details(verbosity=2) @@ -305,4 +332,80 @@ def _get_details(verbosity): details = _get_details(verbosity=1) if len(details) > max_length: details = _get_details(verbosity=0) - return details + return [details] + + # ----------------------------------------------------------------- + + def _commit_entry(self, c, max_length): + # A commit block plus its trailing separator, rendered at the highest + # verbosity (3 -> 1) that fits within max_length. + # In theory, it falls back to the least verbose form if even that is + # too long to fit, but this will never happen in practice (knock on wood) + for verbosity in (3, 2, 1): + entry = self._commit_block(c, verbosity) + "\n" + COMMENT_SEPARATOR + if len(entry) <= max_length: + return entry + return entry + + def _chunk_comments(self, entries, max_length, first_header=""): + # Greedily group the already-rendered commit entries into comment-sized + # strings, each kept under max_length. first_header goes on the first + # comment only. entries is assumed non-empty; the first comment is + # seeded with the first entry, then each subsequent entry starts a new + # comment whenever it would overflow the current one. + comments = [] + current = first_header + COMMENT_SEPARATOR + entries[0] + for entry in entries[1:]: + if len(current) + len(entry) > max_length: + comments.append(current) + current = COMMENT_SEPARATOR + entry + else: + current += entry + comments.append(current) + return comments + + def _build_chained_comment_description(self, list_of_commits, max_length, repo_url): + # Always emit full commit details, split across as many comments as needed. + if not list_of_commits: + return [""] + + # The compare URL must start at the parent of the oldest commit so that + # the oldest commit itself is included in the range (a `A...B` compare + # excludes A). + base_revision = list_of_commits[0].parent_revision + newest_revision = list_of_commits[-1].revision + # The caller has already copied the list; reorder it newest-first. + list_of_commits.reverse() + + # Format each commit at full detail, but drop to a lower verbosity for + # any single commit whose block would not otherwise fit in a comment. + # A comment is a leading separator followed by entries, so budget for it. + entry_budget = max_length - len(COMMENT_SEPARATOR) + entries = [self._commit_entry(c, entry_budget) for c in list_of_commits] + + # If the whole thing fits in one comment, post it as-is, with no header. + whole_length = len(COMMENT_SEPARATOR) + sum(len(e) for e in entries) + if whole_length <= max_length: + return [COMMENT_SEPARATOR + "".join(entries)] + + # It needs multiple comments: head the first one with a link to the + # whole commit range so it can be reviewed at a glance. Passing the + # header to _chunk_comments lets its length count against the budget. + compare_url = repo_and_compare_url(repo_url, base_revision, newest_revision) if (repo_url and base_revision) else None + + if compare_url: + first_header = "All %s commits: %s\n(continued in following comments)\n\n" % (len(list_of_commits), compare_url) + else: + first_header = "" + return self._chunk_comments(entries, max_length, first_header) + + # ----------------------------------------------------------------- + + def build_bug_description(self, list_of_commits, max_length, repo_url=None, options=None): + # The commits are ordered oldest to newest. Copy once here since both + # builders reorder the list in place, then dispatch on the task options. + options = options or {} + list_of_commits = copy.deepcopy(list_of_commits) + if 'verbose-diff' in options: + return self._build_chained_comment_description(list_of_commits, max_length, repo_url) + return self._build_single_comment_description(list_of_commits, max_length) diff --git a/components/utilities.py b/components/utilities.py index 58e1d6a6..860ee9dc 100644 --- a/components/utilities.py +++ b/components/utilities.py @@ -4,6 +4,7 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +import os import copy import inspect import pickle @@ -14,6 +15,20 @@ RETRY_TIMES_OVERRIDE = None +PROMPTS_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "prompts") + + +def load_prompt(name, **substitutions): + """ + Load the prompt template prompts/.md and substitute any + {{ key }} placeholders with the provided keyword arguments. + """ + with open(os.path.join(PROMPTS_DIR, name + ".md")) as f: + text = f.read() + for key, value in substitutions.items(): + text = text.replace("{{ %s }}" % key, value) + return text + class Struct: def __init__(self, **entries): diff --git a/format.sh b/format.sh index daf4f4d7..dbaa4ec9 100755 --- a/format.sh +++ b/format.sh @@ -3,7 +3,7 @@ poetry run autopep8 . poetry run flake8 . -output=$(find . -type f -name '*.py' -exec awk ' +output=$(find . -type d \( -name .venv -o -name venv \) -prune -o -type f -name '*.py' -exec awk ' /^[ \t]*@retry[ \t]*$/ { getline a if (a ~ /^[ \t]*@logEntryExit[ \t]*$/) { diff --git a/localconfig.py.example b/localconfig.py.example index f8f5b4ba..cfadacfb 100644 --- a/localconfig.py.example +++ b/localconfig.py.example @@ -24,6 +24,9 @@ localconfig = { 'Bugzilla': { 'apikey': '' }, + 'AI': { + 'apikey': '' + }, 'Taskcluster': { 'url_treeherder': 'https://treeherder.mozilla.org/', 'url_taskcluster': 'https://firefox-ci-tc.services.mozilla.com/', diff --git a/prompts/conflict_resolution.md b/prompts/conflict_resolution.md new file mode 100644 index 00000000..5277c09f --- /dev/null +++ b/prompts/conflict_resolution.md @@ -0,0 +1,18 @@ +Some third party dependencies have local patches Mozilla applies after vendoring in the third party library. Sometimes those patches do not apply cleanly. This is where you come in. Updatebot has called you into help resolve these conflicts. You should take the following steps. + +At the end you will wrote out a file called conflict_resolution_result.json. It will contain a dictionary with two fields. "outcome" should have a value of "trivial success", "uncertain success", or "failure". "details" will contain an array of strings, where each string is a sentence or paragraph. + +{{ conflict_resolution_instructions }} + +### Conflict Resolution Process + +When a patch does not apply cleanly, for each rejection in the patch: + +1. Determine if the hunk does not apply because the surrounding context has moved or changed, but the intent and general location of the patch relative to other code remains the same. If so, update the .patch file to apply correctly for this hunk, and continue to the next hunk. +2. Determine if the hunk does not apply because it seems like the hunk has already been applied. This can indicate that the patch was upstreamed and it is no longer necessary for this library. Make a note that think hunk is no longer valid and continue trying apply hunks. +3. Finally, if the hunk does not apply cleanly because something has changed in the code that is a non-trivial change, use local tools such as git to understand what change was made upstream, what the intention of the local patch was, and how you can reconcile them. If you are able to do this, continue to the next hunk. In this situation, the final `outcome` value must be either `uncertain success` or `failure`. +4. If you have attempted to understand the problem and are uncertain of how to process, you should stop attempting hunks, and patches, and append a summary of your confusion to the `details` array, and you must return an outcome of `failure`. + +If, at the end, no hunks were applied, this is a good indication the patch was upstreamed. In this case remove the patch file from the repo with `hg rm` or `git rm`, and remove it from the moz.yaml patch list. Make a note of this in the `details` array. Continue to the next patch. + +Alternately, if all the hunks have been updated and applies successfully, make a note of this in the `details` array, and continue to the next patch. diff --git a/prompts/conflict_resolution_details.md b/prompts/conflict_resolution_details.md new file mode 100644 index 00000000..4986676f --- /dev/null +++ b/prompts/conflict_resolution_details.md @@ -0,0 +1,9 @@ +1. Ensure there are no modified or untracked files in the repository. If there are, revert them or remove them. +2. Review the moz.yaml file located at {{ moz_yaml_path }} and specifically the list of patches specified +3. Run `./mach vendor --patch-mode only {{ moz_yaml_path }}` and observe its output +4. Ensure there are no modified or untracked files in the repository. If there are, revert them or remove them. +5. Iterally attempt to apply the patches specified in the moz.yaml using the same technique that was observed from `./mach vendor`. The source code for this process lives in import_local_patches inside python/mozbuild/mozbuild/vendor/vendor_manifest.py in the firefox source directory +6. When you encounter a patch that does not apply, review its contents. Follow the instructions in 'Conflict Resolution Process' below. If you are instructed to continue, do so, continuing steps 5 and 6 for all patches listed in the moz.yaml +7. If any conflict resolution process indicated that the outcome should be failure; make that the `outcome`. If any conflict resolution process indicates that it can be either `uncertain success` or `failure`, then the outcome is `uncertain success`. If you otherwise applied everything simply and successfully the outcome should be `trivial success`. +8. When you have completed, commit _only_ the updates you made to the .patch files and (if applicable) moz.yaml. The commit message for this commit should be "{{ patch_fix_commit_message }}" and it should append the `details` you have been keeping track of. +9. Write the json file with the `outcome` and the `details` array. diff --git a/prompts/system.md b/prompts/system.md new file mode 100644 index 00000000..a2a96d74 --- /dev/null +++ b/prompts/system.md @@ -0,0 +1 @@ +You are the super-intelligent expert inside a system called Updatebot. Updatebot's purpose is to ensure third party dependencies inside Firefox are kept up to date. It runs on a cron job and checks a series of manifests and upstream repositories, checking for the latest releases, downloading them, applying them, and then creating bugs in Mozilla's bugtracker, and sending try runs into the build server. diff --git a/pyproject.toml b/pyproject.toml index f0ce3472..69374fbc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,3 +29,4 @@ build-backend = "poetry.masonry.api" ignore = "E501,E402,E275" in-place = true recursive = true +exclude = ".svn,CVS,.bzr,.hg,.git,__pycache__,.tox,.eggs,*.egg,.venv,venv" diff --git a/tasktypes/commitalert.py b/tasktypes/commitalert.py index 592de037..b08a0fce 100644 --- a/tasktypes/commitalert.py +++ b/tasktypes/commitalert.py @@ -105,7 +105,10 @@ def _process_new_commits(self, library, task, new_commits, all_library_jobs): depends_on = all_library_jobs[0].bugzilla_id if all_library_jobs else None open_dependencies = self.bugzillaProvider.find_open_bugs_info([j.bugzilla_id for j in all_library_jobs]) - description = CommentTemplates.EXAMINE_COMMITS_BODY(library, task, self.scmProvider.build_bug_description(filtered_commits, 65534 - 500), open_dependencies) + commit_chunks = self.scmProvider.build_bug_description(filtered_commits, 65534 - 500, library.repo_url, options=task.options) + description = CommentTemplates.EXAMINE_COMMITS_BODY(library, task, commit_chunks[0], open_dependencies) bugzilla_id = self.bugzillaProvider.file_bug(library, CommentTemplates.EXAMINE_COMMITS_SUMMARY(library, new_commits), description, task.cc, needinfo=task.needinfo, depends_on=depends_on, blocks=task.blocking, moco_confidential=True) + for chunk in commit_chunks[1:]: + self.bugzillaProvider.comment_on_bug(bugzilla_id, chunk) self.dbProvider.create_job(JOBTYPE.COMMITALERT, library, newest_commit.revision, JOBSTATUS.DONE, JOBOUTCOME.ALL_SUCCESS, bugzilla_id) diff --git a/tasktypes/vendoring.py b/tasktypes/vendoring.py index 132845d9..e562d625 100644 --- a/tasktypes/vendoring.py +++ b/tasktypes/vendoring.py @@ -17,6 +17,14 @@ from components.hg import reset_repository +def _patch_exception_message(e): + if isinstance(e, subprocess.CalledProcessError): + msg = ("stderr:\n" + e.stderr.decode().rstrip() + "\n\n") if e.stderr else "" + msg += ("stdout:\n" + e.stdout.decode().rstrip()) if e.stdout else "" + return msg + return str(e) + + class VendorTaskRunner(BaseTaskRunner): def __init__(self, provider_dictionary, config_dictionary): self.jobType = JOBTYPE.VENDORING @@ -138,10 +146,15 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo # File the bug ------------------------ all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job.version if most_recent_job else None) commit_stats = self.mercurialProvider.diff_stats() - commit_details = self.scmProvider.build_bug_description(all_upstream_commits, 65534 - len(commit_stats) - 220) if library.should_show_commit_details else "" + if library.should_show_commit_details: + commit_chunks = self.scmProvider.build_bug_description(all_upstream_commits, 65534 - len(commit_stats) - 220, library.repo_url, options=task.options) + else: + commit_chunks = [""] - created_job.bugzilla_id = self.bugzillaProvider.file_bug(library, CommentTemplates.UPDATE_SUMMARY(library, new_version, timestamp), CommentTemplates.UPDATE_DETAILS(len(all_upstream_commits), len(unseen_upstream_commits), commit_stats, commit_details), task.cc, blocks=task.blocking) + created_job.bugzilla_id = self.bugzillaProvider.file_bug(library, CommentTemplates.UPDATE_SUMMARY(library, new_version, timestamp), CommentTemplates.UPDATE_DETAILS(len(all_upstream_commits), len(unseen_upstream_commits), commit_stats, commit_chunks[0]), task.cc, blocks=task.blocking) self.dbProvider.update_job_add_bug_id(created_job, created_job.bugzilla_id) + for chunk in commit_chunks[1:]: + self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, chunk) # Address any prior bug --------------- if most_recent_job and not most_recent_job.relinquished: @@ -172,19 +185,49 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, CommentTemplates.COULD_NOT_GENERAL_ERROR("commit the updated library."), needinfo=library.maintainer_bz) raise e + ai_resolved_conflicts = False if library.has_patches: # Apply Patches ------------------- try: self.vendorProvider.patch(library, new_version) except Exception as e: - if isinstance(e, subprocess.CalledProcessError): - msg = ("stderr:\n" + e.stderr.decode().rstrip() + "\n\n") if e.stderr else "" - msg += ("stdout:\n" + e.stdout.decode().rstrip()) if e.stdout else "" - else: - msg = str(e) - self.dbProvider.update_job_status(created_job, JOBSTATUS.DONE, JOBOUTCOME.COULD_NOT_PATCH) - self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, CommentTemplates.COULD_NOT_GENERAL_ERROR("apply the mozilla patches.", errormessage=msg), needinfo=library.maintainer_bz) - return + msg = _patch_exception_message(e) + + # The mozilla patches didn't apply cleanly. Ask the AI to try to + # resolve the conflicts (updating the .patch files / moz.yaml). + potential_commit_message = "Bug %s - Update %s local patches to apply cleanly" % ( + created_job.bugzilla_id, library.name) + resolution = self.aiProvider.resolve_patch_conflicts( + library.yaml_path, potential_commit_message, cwd=self.config['General'].get('gecko-path'), + library_name=library.name, job_id=created_job.id) + outcome = resolution.get("outcome") if resolution else "failure" + ai_details = "\n".join(resolution.get("details", [])) if resolution else "" + + if outcome == "failure": + self.dbProvider.update_job_status(created_job, JOBSTATUS.DONE, JOBOUTCOME.COULD_NOT_PATCH) + self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, CommentTemplates.COULD_NOT_GENERAL_ERROR_WITH_AI("apply the mozilla patches.", initialerrormessage=msg, ai_outcome=outcome, ai_details=ai_details), needinfo=library.maintainer_bz) + return + + # The AI reports it resolved the conflicts (and committed the + # updated .patch files). Record the outcome on the bug, flagging + # the maintainer for review when it's only 'uncertain success'. + needinfo = library.maintainer_bz if outcome == "uncertain success" else None + self.bugzillaProvider.comment_on_bug( + created_job.bugzilla_id, + CommentTemplates.AI_RESOLVED_PATCH_CONFLICTS(outcome, ai_details), + needinfo=needinfo) + + # Re-apply the now-updated patches so the vendored tree is patched. + try: + self.vendorProvider.patch(library, new_version) + except Exception as e2: + self.dbProvider.update_job_status(created_job, JOBSTATUS.DONE, JOBOUTCOME.COULD_NOT_PATCH) + self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, CommentTemplates.COULD_NOT_GENERAL_ERROR("apply the mozilla patches even after AI conflict resolution.", errormessage=_patch_exception_message(e2)), needinfo=library.maintainer_bz) + return + + # The AI added its own commit for the .patch/moz.yaml fixes, so + # there is now an extra commit to submit to Phabricator. + ai_resolved_conflicts = True # Commit Patches ------------------ try: self.mercurialProvider.commit_patches(library, created_job.bugzilla_id, new_version) @@ -209,11 +252,19 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo # Submit Phab Revision ---------------- try: - phab_revisions = self.phabricatorProvider.submit_patches(created_job.bugzilla_id, library.has_patches) - assert len(phab_revisions) == 2 if library.has_patches else 1, "We don't have the correct number of phabricator patches; we have %s, expected %s" % (len(phab_revisions), 2 if library.has_patches else 1) - self.dbProvider.add_phab_revision(created_job, phab_revisions[0], 'vendoring commit') - if len(phab_revisions) > 1: - self.dbProvider.add_phab_revision(created_job, phab_revisions[1], 'patches commit') + # One commit for the vendoring, plus one for the patches (if any), + # plus one more if the AI added a commit resolving patch conflicts. + num_commits = 1 + (1 if library.has_patches else 0) + (1 if ai_resolved_conflicts else 0) + phab_revisions = self.phabricatorProvider.submit_patches(created_job.bugzilla_id, num_commits) + assert len(phab_revisions) == num_commits, "We don't have the correct number of phabricator patches; we have %s, expected %s" % (len(phab_revisions), num_commits) + # Revisions come back bottom-up, matching the order the commits were made. + purposes = ['vendoring commit'] + if ai_resolved_conflicts: + purposes.append('patch conflict resolution commit') + if library.has_patches: + purposes.append('patches commit') + for revision, purpose in zip(phab_revisions, purposes): + self.dbProvider.add_phab_revision(created_job, revision, purpose) except Exception as e: self.dbProvider.update_job_status(created_job, JOBSTATUS.DONE, JOBOUTCOME.COULD_NOT_SUBMIT_TO_PHAB) self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, CommentTemplates.COULD_NOT_GENERAL_ERROR("submit to phabricator."), needinfo=library.maintainer_bz) diff --git a/test.py b/test.py index ed608d16..e076c3b6 100755 --- a/test.py +++ b/test.py @@ -19,6 +19,8 @@ "python_language", "treeherder_api", "library", + "build_bug_description", + "aiprovider", "lambda_capture", "class_passing", "frequency" diff --git a/test_local.py b/test_local.py new file mode 100755 index 00000000..bdb32bea --- /dev/null +++ b/test_local.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +""" +Runner for the local, live test tier -- NOT run by test.py / CI. + +These tests drive real external backends (e.g. the `claude` CLI) against a real +gecko checkout, so they cost money, are non-deterministic, and are opt-in. Each +test SKIPS if its requirements (api keys, a gecko checkout, CLIs) aren't present. + +The tests themselves live in tests/local-tests/. That directory name is not a +valid Python package (the hyphen), so we add it to sys.path and import the test +modules by file name, rather than as tests.local-tests.* . + +Usage: + poetry run ./test_local.py +""" + +import os +import sys +import unittest +import importlib + +# Repo root (for `components.*`, `apis.*`, localconfig, ...) and the local-tests +# directory (for the test modules and their fixtures) both need to be importable. +sys.path.append(".") +LOCAL_TESTS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "tests", "local-tests") +sys.path.insert(0, LOCAL_TESTS_DIR) + +# Must correspond to the file names in tests/local-tests/ +LOCAL_TESTS = [ + "ai_conflict_resolution", + "full_vendor_run_nestegg", +] + +modules = [importlib.import_module(t) for t in LOCAL_TESTS] + +loader = unittest.TestLoader() +suite = unittest.TestSuite() +for m in modules: + suite.addTests(loader.loadTestsFromModule(m)) + +# buffer=False: unlike the deterministic suite, the point of these live tests is +# to watch the real run (provider logs, the AI's reported details) as it happens. +if unittest.TextTestRunner(verbosity=3, buffer=False).run(suite).wasSuccessful(): + exit(0) +else: + exit(1) diff --git a/tests/aiprovider.py b/tests/aiprovider.py new file mode 100755 index 00000000..0ef3d40f --- /dev/null +++ b/tests/aiprovider.py @@ -0,0 +1,159 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import os +import sys +import json +import tempfile +import unittest + +sys.path.append(".") +sys.path.append("..") + +from components.aiprovider import AIProvider, CONFLICT_RESOLUTION_RESULT_FILE +from components.utilities import Struct, load_prompt +from components.logging import SimpleLogger + + +class RecordingCommandProvider: + """ + Stands in for the CommandProvider: records how it was invoked (args, cwd, + stdin, env), returns canned `claude` JSON, and optionally writes a + conflict_resolution_result.json into cwd the way the real CLI would. + """ + + def __init__(self, stdout="{}", returncode=0, result_file_contents=None): + self.stdout = stdout + self.returncode = returncode + self.result_file_contents = result_file_contents + self.calls = [] + + def run(self, args, shell=False, clean_return=True, cwd=None, stdin_path=None, timeout=60 * 20, env=None): + call = {"args": args, "cwd": cwd, "stdin_path": stdin_path, "env": env, + "prompt": None, "system_prompt": None} + if stdin_path and os.path.exists(stdin_path): + with open(stdin_path) as f: + call["prompt"] = f.read() + if "--append-system-prompt-file" in args: + system_path = args[args.index("--append-system-prompt-file") + 1] + if os.path.exists(system_path): + with open(system_path) as f: + call["system_prompt"] = f.read() + self.calls.append(call) + if self.result_file_contents is not None and cwd: + with open(os.path.join(cwd, CONFLICT_RESOLUTION_RESULT_FILE), "w") as f: + f.write(self.result_file_contents) + return Struct(**{"stdout": self.stdout.encode(), + "returncode": self.returncode, + "check_returncode": lambda: None}) + + +def make_ai(config, command_provider): + ai = AIProvider(config) + ai.update_config({ + "CommandProvider": command_provider, + "LoggingProvider": SimpleLogger({"local": False}), + }) + return ai + + +class TestAIProviderParseResult(unittest.TestCase): + def _parse(self, stdout, returncode): + ai = make_ai({}, RecordingCommandProvider()) + return ai._parse_result(Struct(**{"stdout": stdout, "returncode": returncode, + "check_returncode": lambda: None})) + + def test_success(self): + r = self._parse(b'{"is_error": false, "result": "done", "session_id": "abc"}', 0) + self.assertTrue(r.success) + self.assertEqual(r.text, "done") + self.assertEqual(r.session_id, "abc") + + def test_is_error_true(self): + r = self._parse(b'{"is_error": true, "result": "nope"}', 0) + self.assertFalse(r.success) + + def test_nonzero_returncode(self): + r = self._parse(b'{"is_error": false, "result": "x"}', 1) + self.assertFalse(r.success) + + def test_malformed_json(self): + r = self._parse(b'this is not json', 0) + self.assertFalse(r.success) + + +class TestAIProviderResolvePatchConflicts(unittest.TestCase): + def test_success_returns_dict_and_uses_expected_invocation(self): + canned = json.dumps({"outcome": "trivial success", "details": ["updated foo.patch"]}) + cmd = RecordingCommandProvider(stdout='{"is_error": false, "result": "ok"}', + result_file_contents=canned) + ai = make_ai({"apikey": "sk-test", "model": "claude-test-model"}, cmd) + + with tempfile.TemporaryDirectory() as d: + resolution = ai.resolve_patch_conflicts("media/libfoo/moz.yaml", "Bug 1 - fix patches", cwd=d) + # The result file is consumed (removed) after being read. + self.assertFalse(os.path.exists(os.path.join(d, CONFLICT_RESOLUTION_RESULT_FILE))) + + self.assertEqual(resolution["outcome"], "trivial success") + self.assertEqual(resolution["details"], ["updated foo.patch"]) + + call = cmd.calls[0] + self.assertEqual(call["args"][0], "claude") + self.assertIn("-p", call["args"]) + self.assertIn("--permission-mode", call["args"]) + self.assertIn("bypassPermissions", call["args"]) + self.assertIn("--append-system-prompt-file", call["args"]) + self.assertIn("claude-test-model", call["args"]) + self.assertEqual(call["env"], {"ANTHROPIC_API_KEY": "sk-test"}) + # The prompt is fed over stdin and rendered from the templates. + self.assertIsNotNone(call["prompt"]) + self.assertIn("media/libfoo/moz.yaml", call["prompt"]) + self.assertIn("Bug 1 - fix patches", call["prompt"]) + self.assertIn("Conflict Resolution Process", call["prompt"]) + # The system prompt is prompts/system.md. + self.assertIn("Updatebot", call["system_prompt"]) + + def test_missing_result_file_returns_none(self): + cmd = RecordingCommandProvider(stdout='{"is_error": false}', result_file_contents=None) + ai = make_ai({"apikey": "sk-test"}, cmd) + with tempfile.TemporaryDirectory() as d: + self.assertIsNone(ai.resolve_patch_conflicts("m/moz.yaml", "msg", cwd=d)) + + def test_garbage_result_file_returns_none(self): + cmd = RecordingCommandProvider(stdout='{"is_error": false}', result_file_contents="not json") + ai = make_ai({"apikey": "sk-test"}, cmd) + with tempfile.TemporaryDirectory() as d: + self.assertIsNone(ai.resolve_patch_conflicts("m/moz.yaml", "msg", cwd=d)) + + def test_no_apikey_passes_no_env(self): + canned = json.dumps({"outcome": "failure", "details": []}) + cmd = RecordingCommandProvider(stdout='{"is_error": false}', result_file_contents=canned) + ai = make_ai({}, cmd) # no apikey configured + with tempfile.TemporaryDirectory() as d: + ai.resolve_patch_conflicts("m/moz.yaml", "msg", cwd=d) + self.assertIsNone(cmd.calls[0]["env"]) + + +class TestLoadPrompt(unittest.TestCase): + def test_substitution(self): + p = load_prompt("conflict_resolution_details", + moz_yaml_path="media/libx/moz.yaml", + patch_fix_commit_message="Bug 2 - update patches") + self.assertIn("media/libx/moz.yaml", p) + self.assertIn("Bug 2 - update patches", p) + self.assertNotIn("{{ moz_yaml_path }}", p) + self.assertNotIn("{{ patch_fix_commit_message }}", p) + + def test_composition(self): + details = load_prompt("conflict_resolution_details", + moz_yaml_path="X", patch_fix_commit_message="M") + p = load_prompt("conflict_resolution", conflict_resolution_instructions=details) + self.assertNotIn("{{ conflict_resolution_instructions }}", p) + self.assertIn("Conflict Resolution Process", p) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/tests/automation_configuration.py b/tests/automation_configuration.py index 677a02eb..e71d2f2b 100755 --- a/tests/automation_configuration.py +++ b/tests/automation_configuration.py @@ -115,6 +115,15 @@ def _update_config(self, config): self.also_expected = "Made it!" +class TestConfigAIProvider(BaseTestConfigProvider): + def __init__(self, config): + self.expected = 'ai!' + super(TestConfigAIProvider, self).__init__(config) + + def _update_config(self, config): + self.also_expected = "Made it!" + + class TestCommandRunner(unittest.TestCase): def testConfigurationPassing(self): configs = { @@ -134,6 +143,7 @@ def testConfigurationPassing(self): 'Logging': {'specialkey': 'logging!'}, 'Library': {'specialkey': 'library!'}, 'SCM': {'specialkey': 'scm!'}, + 'AI': {'specialkey': 'ai!'}, } providers = { 'Database': TestConfigDatabaseProvider, @@ -145,7 +155,8 @@ def testConfigurationPassing(self): 'Logging': TestConfigLoggingProvider, 'Command': TestConfigCommandProvider, 'Library': TestConfigLibraryProvider, - 'SCM': TestConfigSCMProvider + 'SCM': TestConfigSCMProvider, + 'AI': TestConfigAIProvider } u = Updatebot(configs, providers) diff --git a/tests/build_bug_description.py b/tests/build_bug_description.py new file mode 100644 index 00000000..530bc66f --- /dev/null +++ b/tests/build_bug_description.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import sys +import unittest + +sys.path.append(".") +sys.path.append("..") + +from components.scmprovider import SCMProvider, Commit + +GITHUB_REPO = "https://github.com/org/repo" + + +def make_commit(revision, parent_revision, summary="Do a thing", + description="A longer description of the thing.", + files_modified=None): + # Build a fully-populated Commit without touching a real repository. + c = Commit("%s|2020-01-01 00:00:00 +0000|2020-01-02 00:00:00 +0000" % revision) + c.summary = summary + c.author = "Some Developer " + c.description = description + c.revision_link = "%s/commit/%s" % (GITHUB_REPO, revision) + c.parent_revision = parent_revision + c.files_modified = files_modified if files_modified is not None else ["src/thing.c"] + c.populated = True + return c + + +class TestBuildBugDescription(unittest.TestCase): + @classmethod + def setUpClass(cls): + # build_bug_description / _commit_block don't use the command or logging + # providers, so a bare provider is enough. + cls.scm = SCMProvider({}) + + def _commits(self): + # Ordered oldest -> newest, as build_bug_description expects. + return [ + make_commit("aaaaaaa", "p0000000", summary="First commit"), + make_commit("bbbbbbb", "aaaaaaa", summary="Second commit"), + make_commit("ccccccc", "bbbbbbb", summary="Third commit"), + ] + + # -- _commit_block --------------------------------------------------------- + + def test_commit_block_verbosity_levels(self): + c = make_commit("abcdef0", "0fedcba", summary="Fix the widget", + description="Body of the change.", files_modified=["src/w.c"]) + + v1 = self.scm._commit_block(c, 1) + self.assertIn("abcdef0 by Some Developer", v1) + self.assertIn(c.revision_link, v1) + self.assertNotIn("Authored:", v1) + self.assertNotIn("Fix the widget", v1) + self.assertNotIn("Files Modified", v1) + + v2 = self.scm._commit_block(c, 2) + self.assertIn("Authored:", v2) + self.assertIn("Committed:", v2) + self.assertIn("Fix the widget", v2) + self.assertNotIn("Body of the change.", v2) + self.assertNotIn("Files Modified", v2) + + v3 = self.scm._commit_block(c, 3) + self.assertIn("Fix the widget", v3) + self.assertIn("Body of the change.", v3) + self.assertIn("Files Modified", v3) + self.assertIn("src/w.c", v3) + + def test_commit_block_requires_populated(self): + c = Commit("deadbee|d|d") # not populated + with self.assertRaises(Exception): + self.scm._commit_block(c, 1) + + # -- dispatch -------------------------------------------------------------- + + def test_dispatch_selects_builder(self): + commits = self._commits() + + # Tiny budget makes the two behaviors clearly distinguishable: the + # single-comment builder elides, the chained builder splits instead. + single = self.scm.build_bug_description(commits, 60, GITHUB_REPO, options={}) + self.assertEqual(len(single), 1) + self.assertIn("commits elided", single[0]) + + chained = self.scm.build_bug_description(commits, 60, GITHUB_REPO, options={"verbose-diff": True}) + self.assertGreater(len(chained), 1) + self.assertNotIn("commits elided", "".join(chained)) + + def test_options_default_is_single_comment(self): + commits = self._commits() + # No options passed at all -> single-comment behavior. + result = self.scm.build_bug_description(commits, 60, GITHUB_REPO) + self.assertEqual(len(result), 1) + self.assertIn("commits elided", result[0]) + + # -- single-comment behavior ---------------------------------------------- + + def test_single_comment_full_when_it_fits(self): + commits = self._commits() + result = self.scm.build_bug_description(commits, 65534, GITHUB_REPO, options={}) + self.assertEqual(len(result), 1) + self.assertNotIn("commits elided", result[0]) + # Full detail present, newest commit first. + self.assertIn("Third commit", result[0]) + self.assertIn("Files Modified", result[0]) + self.assertLess(result[0].index("Third commit"), result[0].index("First commit")) + + # -- chained behavior ------------------------------------------------------ + + def test_chained_single_chunk_when_it_fits(self): + commits = self._commits() + result = self.scm.build_bug_description(commits, 65534, GITHUB_REPO, options={"verbose-diff": True}) + self.assertEqual(len(result), 1) + # No continuation header for a single chunk. + self.assertNotIn("continued in following comments", result[0]) + + def test_chained_splits_with_compare_url_including_oldest(self): + commits = self._commits() + result = self.scm.build_bug_description(commits, 200, GITHUB_REPO, options={"verbose-diff": True}) + self.assertGreater(len(result), 1) + # The compare range starts at the parent of the oldest commit, so the + # oldest commit itself is included. + self.assertIn("All 3 commits: %s/compare/p0000000...ccccccc" % GITHUB_REPO, result[0]) + self.assertIn("continued in following comments", result[0]) + + def test_chained_no_compare_url_for_unsupported_host(self): + commits = self._commits() + result = self.scm.build_bug_description(commits, 200, "https://example.com/repo", options={"verbose-diff": True}) + self.assertGreater(len(result), 1) + self.assertNotIn("/compare/", "".join(result)) + + def test_chained_reduces_verbosity_for_oversized_commit(self): + # A single commit whose full detail exceeds the limit is rendered at a + # lower verbosity so it fits, rather than blowing past the comment size. + big = make_commit("abc1234", "par0000", summary="Summary line", + description="D" * 500, files_modified=["src/one.c", "src/two.c"]) + result = self.scm.build_bug_description([big], 200, GITHUB_REPO, options={"verbose-diff": True}) + self.assertEqual(len(result), 1) + self.assertLessEqual(len(result[0]), 200) # it now fits within the limit + self.assertIn("abc1234 by", result[0]) # still identifies the commit + self.assertNotIn("D" * 500, result[0]) # full description dropped + self.assertNotIn("Files Modified", result[0]) # file list dropped + + def test_chained_empty_commit_list(self): + result = self.scm.build_bug_description([], 65534, GITHUB_REPO, options={"verbose-diff": True}) + self.assertEqual(result, [""]) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/tests/functionality_all_platforms.py b/tests/functionality_all_platforms.py index 82d33513..9c2269ec 100755 --- a/tests/functionality_all_platforms.py +++ b/tests/functionality_all_platforms.py @@ -34,6 +34,7 @@ from tests.functionality_utilities import SHARED_COMMAND_MAPPINGS, TRY_OUTPUT, TRY_LOCKED_OUTPUT, ARC_OUTPUT, CONDUIT_EDIT_OUTPUT, MockedBugzillaProvider, treeherder_response from tests.mock_commandprovider import TestCommandProvider +from tests.mock_aiprovider import MockAIProvider from tests.mock_libraryprovider import MockLibraryProvider from tests.mock_treeherder_server import MockTreeherderServerFactory, TYPE_HEALTH from tests.database import transform_db_config_to_tmp_db @@ -85,6 +86,9 @@ def COMMAND_MAPPINGS(expected_values, command_callbacks): # Not Mocked At All 'Phabricator': PhabricatorProvider, 'SCM': SCMProvider, + # Fully Mocked, so we never shell out to a real AI CLI. Defaults to a + # 'failure' outcome; individual tests override via the ai_config parameter. + 'AI': MockAIProvider, } @@ -100,6 +104,7 @@ def _setup(self, assert_prior_bug_reference=True, assert_assignee_func=None, command_callbacks={}, + ai_config=None, keep_tmp_db=False): self.server = server.HTTPServer(('', 27490), MockTreeherderServerFactory(treeherder_response)) t = Thread(target=self.server.serve_forever) @@ -132,6 +137,7 @@ def _setup(self, 'url_taskcluster': 'http://localhost:27490/', }, 'Phabricator': {}, + 'AI': ai_config or {}, 'Library': { 'vendoring_revision_override': "_current", } @@ -378,6 +384,115 @@ def testFailsDuringPatching(self): finally: self._cleanup(u, expected_values) + # Create -> patch fails -> AI resolves the conflict -> re-apply succeeds -> proceeds + @logEntryExitHeaderLine + def testPatchConflictResolvedByAI(self): + patch_calls = [0] + + def patch_callback(): + patch_calls[0] += 1 + if patch_calls[0] == 1: + raise_(Exception("patch does not apply cleanly")) + return "" + + arc_calls = [0] + + def phab_submit(): + arc_calls[0] += 1 + v = 90000 + arc_calls[0] + return ARC_OUTPUT % (v, v) + + library_filter = 'png' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["try_rev|2021-02-09 15:30:04 -0500|2021-02-12 17:40:01 +0000"], + lambda: 51, # get_filed_bug_id_func + lambda b: {}, # filed_bug_ids_func + AssertFalse, # treeherder_response + command_callbacks={'patch': patch_callback, 'phab_submit': phab_submit}, + ai_config={'ai_outcome': 'trivial success', + 'ai_details': ['Updated the local patch to match the refactored upstream code.']}, + ) + try: + u.run(library_filter=library_filter) + + self.assertEqual(u.aiProvider.resolve_call_count, 1, "The AI conflict resolver should have been called once") + self.assertEqual(patch_calls[0], 2, "The patches should have been re-applied after AI resolution") + + lib = [lib for lib in u.libraryProvider.get_libraries(u.config_dictionary['General']['gecko-path']) if library_filter in lib.name][0] + j = u.dbProvider.get_job(lib, expected_values.library_new_version_id()) + self.assertEqual(JOBSTATUS.AWAITING_SECOND_PLATFORMS_TRY_RESULTS, j.status) + self.assertEqual(JOBOUTCOME.PENDING, j.outcome) + # Three phabricator revisions now: vendoring, the AI patch-fix, and the patches commit. + self.assertEqual(len(j.phab_revisions), 3, "Expected 3 phabricator revisions after AI conflict resolution") + self.assertEqual([p.purpose for p in j.phab_revisions], + ['vendoring commit', 'patch conflict resolution commit', 'patches commit']) + finally: + self._cleanup(u, expected_values) + + # Same as above but the AI is only 'uncertain' about the resolution; the job still proceeds. + @logEntryExitHeaderLine + def testPatchConflictResolvedByAIUncertain(self): + patch_calls = [0] + + def patch_callback(): + patch_calls[0] += 1 + if patch_calls[0] == 1: + raise_(Exception("patch does not apply cleanly")) + return "" + + arc_calls = [0] + + def phab_submit(): + arc_calls[0] += 1 + v = 91000 + arc_calls[0] + return ARC_OUTPUT % (v, v) + + library_filter = 'png' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["try_rev|2021-02-09 15:30:04 -0500|2021-02-12 17:40:01 +0000"], + lambda: 53, # get_filed_bug_id_func + lambda b: {}, # filed_bug_ids_func + AssertFalse, # treeherder_response + command_callbacks={'patch': patch_callback, 'phab_submit': phab_submit}, + ai_config={'ai_outcome': 'uncertain success', + 'ai_details': ['Reconciled the patch but the surrounding code changed non-trivially; please review.']}, + ) + try: + u.run(library_filter=library_filter) + self.assertEqual(u.aiProvider.resolve_call_count, 1) + lib = [lib for lib in u.libraryProvider.get_libraries(u.config_dictionary['General']['gecko-path']) if library_filter in lib.name][0] + j = u.dbProvider.get_job(lib, expected_values.library_new_version_id()) + self.assertEqual(JOBSTATUS.AWAITING_SECOND_PLATFORMS_TRY_RESULTS, j.status) + self.assertEqual(JOBOUTCOME.PENDING, j.outcome) + self.assertEqual(len(j.phab_revisions), 3) + finally: + self._cleanup(u, expected_values) + + # patch fails -> AI claims success -> but the patches STILL fail to re-apply -> COULD_NOT_PATCH + @logEntryExitHeaderLine + def testPatchConflictAIResolvedButReapplyStillFails(self): + library_filter = 'png' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["try_rev|2021-02-09 15:30:04 -0500|2021-02-12 17:40:01 +0000"], + lambda: 54, # get_filed_bug_id_func + lambda b: {}, # filed_bug_ids_func + AssertFalse, # treeherder_response + command_callbacks={'patch': lambda: raise_(Exception("patch still does not apply"))}, + ai_config={'ai_outcome': 'trivial success', 'ai_details': ['I think I fixed it.']}, + ) + try: + u.run(library_filter=library_filter) + self.assertEqual(u.aiProvider.resolve_call_count, 1) + lib = [lib for lib in u.libraryProvider.get_libraries(u.config_dictionary['General']['gecko-path']) if library_filter in lib.name][0] + j = u.dbProvider.get_job(lib, expected_values.library_new_version_id()) + self.assertEqual(JOBSTATUS.DONE, j.status) + self.assertEqual(JOBOUTCOME.COULD_NOT_PATCH, j.outcome) + finally: + self._cleanup(u, expected_values) + # Create -> ./mach vendor -> commit -> mach vendor patch -> Fails during committing @logEntryExitHeaderLine def testFailsDuringPatchingCommit(self): diff --git a/tests/functionality_commitalert.py b/tests/functionality_commitalert.py index 79504b60..da6ecdfa 100755 --- a/tests/functionality_commitalert.py +++ b/tests/functionality_commitalert.py @@ -26,6 +26,7 @@ from components.commandprovider import CommandProvider from tests.mock_commandprovider import TestCommandProvider, DO_EXECUTE +from tests.mock_aiprovider import MockAIProvider from tests.mock_libraryprovider import MockLibraryProvider from tests.mock_repository import COMMITS_BRANCH1, COMMITS_MAIN from tests.database import transform_db_config_to_tmp_db @@ -128,6 +129,8 @@ def bug_has_landing_link(self, bug_id): 'Library': MockLibraryProvider, # Not mocked 'SCM': SCMProvider, + # Fully Mocked (commit-alert tasks don't use it, but keep it off the real CLI). + 'AI': MockAIProvider, 'Mercurial': NeverUseMeClass, 'Taskcluster': NeverUseMeClass, 'Vendor': NeverUseMeClass, diff --git a/tests/functionality_two_platforms.py b/tests/functionality_two_platforms.py index a24677c7..b4e0b82b 100755 --- a/tests/functionality_two_platforms.py +++ b/tests/functionality_two_platforms.py @@ -34,6 +34,7 @@ from tests.functionality_utilities import SHARED_COMMAND_MAPPINGS, TRY_OUTPUT, TRY_LOCKED_OUTPUT, CONDUIT_EDIT_OUTPUT, MockedBugzillaProvider, treeherder_response from tests.mock_commandprovider import TestCommandProvider +from tests.mock_aiprovider import MockAIProvider from tests.mock_libraryprovider import MockLibraryProvider from tests.mock_treeherder_server import MockTreeherderServerFactory, TYPE_HEALTH from tests.database import transform_db_config_to_tmp_db @@ -91,7 +92,9 @@ def COMMAND_MAPPINGS(expected_values, command_callbacks): 'Taskcluster': TaskclusterProvider, # Not Mocked At All 'Phabricator': PhabricatorProvider, - 'SCM': SCMProvider + 'SCM': SCMProvider, + # Fully Mocked; defaults to a 'failure' outcome for patch conflicts. + 'AI': MockAIProvider } diff --git a/tests/functionality_utilities.py b/tests/functionality_utilities.py index 5ad07cb0..bae4093c 100644 --- a/tests/functionality_utilities.py +++ b/tests/functionality_utilities.py @@ -86,6 +86,7 @@ def default_phab_submit(): ("git log -1 --oneline", lambda: "0481f1c (HEAD -> issue-115-add-revision-to-log, origin/issue-115-add-revision-to-log) Issue #115 - Add revision of updatebot to log output"), ("git clone https://example.invalid .", lambda: ""), ("git merge-base", lambda: "_current"), + ("git rev-parse", lambda: "abcdef1234567890abcdef1234567890abcdef12"), ("git log --pretty=%H|%ai|%ci", lambda cmd: "\n".join(expected_values.git_pretty_output_func("_current" not in cmd))), ("git diff --name-status", lambda: GIT_DIFF_FILES_CHANGES), ("git log --pretty=%s", lambda: "Roll SPIRV-Tools from a61d07a72763 to 1cda495274bb (1 revision)"), diff --git a/tests/local-tests/ai_conflict_resolution.py b/tests/local-tests/ai_conflict_resolution.py new file mode 100755 index 00000000..82b3efb7 --- /dev/null +++ b/tests/local-tests/ai_conflict_resolution.py @@ -0,0 +1,248 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +""" +Local, live test: AIProvider.resolve_patch_conflicts against a *real* AI backend. + +This is NOT run by test.py / CI. It drives the real `claude` CLI on a real gecko +checkout, using the committed nestegg conflict scenario. It costs money and is +non-deterministic, so it is opt-in and lives in the local-tests tier (run it with +`poetry run ./test_local.py`). + +Requirements (from localconfig.py or the environment); the test SKIPS if any are +missing: + - an AI api key: localconfig['AI']['apikey'] or $ANTHROPIC_API_KEY + - a gecko checkout: localconfig['General']['gecko-path'] or $LIVE_GECKO_PATH + - the `claude` CLI on $PATH + +The gecko checkout MUST be clean (no modifications, no untracked files) before +starting -- we assert this so we never clobber your work. We set up the scenario +as a commit, run the live resolution, assert on the result, and strip everything +back to the original revision (registered as a cleanup up front, so it runs even +if setup fails partway) so the test is re-runnable. +""" + +import os +import re +import sys +import shutil +import subprocess +import tempfile +import unittest + + +def hg(args, cwd): + return subprocess.run(["hg"] + args, cwd=cwd, capture_output=True, text=True) + + +# --- The nestegg conflict scenario ------------------------------------------- +# +# We add a local Mozilla patch to media/libnestegg that was written against an +# older revision of ne_read_uint, so it no longer applies to the current +# (vendored) nestegg.c. Running `./mach vendor --patch-mode only` on it then +# fails with a conflict -- exactly the situation resolve_patch_conflicts is +# meant to resolve. The patch fixture (nestegg_conflict.patch) lives next to +# this file. + +LIBRARY_DIR = "media/libnestegg" +MOZ_YAML = os.path.join(LIBRARY_DIR, "moz.yaml") +PATCH_REL = "patches/0001-clamp-nestegg-uint-reads.patch" +PATCH_FIXTURE = os.path.join(os.path.dirname(os.path.abspath(__file__)), "nestegg_conflict.patch") +SCENARIO_COMMIT_MESSAGE = "TEST SCENARIO (do not land): nestegg local patch that conflicts with upstream" + + +def _add_patch_to_moz_yaml(moz_yaml_path): + with open(moz_yaml_path) as f: + lines = f.readlines() + + if any(PATCH_REL in line for line in lines): + return # already set up + + out = [] + inserted = False + for line in lines: + out.append(line) + # Insert a patches list immediately after the vendor-directory line, + # which sits inside the `vendoring:` block. + if not inserted and line.strip().startswith("vendor-directory:"): + out.append(" patches:\n") + out.append(" - %s\n" % PATCH_REL) + inserted = True + + if not inserted: + raise Exception("Could not find a vendor-directory line to anchor the patches list in %s" % moz_yaml_path) + + with open(moz_yaml_path, "w") as f: + f.writelines(out) + + +def _setup_nestegg_scenario(gecko_path): + """Add the conflicting local patch, list it in moz.yaml, and commit it.""" + lib = os.path.join(gecko_path, LIBRARY_DIR) + os.makedirs(os.path.join(lib, "patches"), exist_ok=True) + + with open(PATCH_FIXTURE) as src: + patch_text = src.read() + with open(os.path.join(lib, PATCH_REL), "w") as dst: + dst.write(patch_text) + + _add_patch_to_moz_yaml(os.path.join(gecko_path, MOZ_YAML)) + + hg(["add", os.path.join(LIBRARY_DIR, PATCH_REL)], gecko_path) + hg(["commit", "-m", SCENARIO_COMMIT_MESSAGE], gecko_path) + + +def restore_checkout(gecko_path, original_rev): + # Restore the checkout: discard working changes, strip everything committed on + # top of original_rev (the scenario setup + anything updatebot/the AI added), + # and purge leftover untracked files, so re-runs start clean. Run every step + # even if an earlier one fails, so a single error doesn't leave a commit + # checked out / the tree dirty. + print("Restoring the checkout to %s ..." % original_rev[:12]) + for step in (["update", "-C", original_rev], + ["debugstrip", "-r", "children(%s)" % original_rev, "--no-backup"], + ["--config", "extensions.purge=", "purge", "--all"]): + try: + r = hg(step, gecko_path) + if r.returncode != 0: + print("WARNING: `hg %s` exited %d:\n%s" % (" ".join(step), r.returncode, r.stderr.strip())) + except OSError as e: + print("WARNING: could not run `hg %s`: %s" % (" ".join(step), e)) + + +def _working_parent_node(gecko_path): + # Return the single 40-char node of the working-directory parent. + # + # We deliberately do NOT trust `hg log -r . -T {node}` blindly: a user's hg + # config (a `log` alias, `[defaults]`, or firefoxtree/version-control-tools + # setup) can turn that into a follow/all-revisions log, so it emits many + # concatenated node hashes -- a multi-megabyte string. Interpolated into the + # teardown revset, that argv exceeds the kernel's ARG_MAX and the shell-out + # dies with `OSError: [Errno 7] Argument list too long`. `-l 1` caps the + # output to one changeset, and we validate the shape before using it. + out = hg(["log", "-r", ".", "-l", "1", "-T", "{node}\n"], gecko_path).stdout + lines = out.splitlines() + node = lines[0].strip() if lines else "" + if not re.fullmatch(r"[0-9a-f]{40}", node): + raise AssertionError( + "Could not determine a single working-parent revision: `hg log -r .` " + "returned %d line(s) / %d chars (first 60: %r). Check whether your hg " + "configuration redefines `log`." % (len(lines), len(out), node[:60])) + return node + + +def _resolve_requirements(): + """Return (apikey, gecko_path), or a string describing what's missing.""" + apikey = os.environ.get("ANTHROPIC_API_KEY") + gecko_path = os.environ.get("LIVE_GECKO_PATH") + try: + from localconfig import localconfig + apikey = apikey or localconfig.get("AI", {}).get("apikey") + gecko_path = gecko_path or localconfig.get("General", {}).get("gecko-path") + except ImportError: + pass + + missing = [] + if not apikey: + missing.append("an AI api key (localconfig['AI']['apikey'] or $ANTHROPIC_API_KEY)") + if not gecko_path: + missing.append("a gecko checkout (localconfig['General']['gecko-path'] or $LIVE_GECKO_PATH)") + if not shutil.which("claude"): + missing.append("the 'claude' CLI on $PATH") + if missing: + return "Missing required configuration:\n - " + "\n - ".join(missing) + + gecko_path = os.path.abspath(gecko_path) + if not os.path.isdir(os.path.join(gecko_path, ".hg")): + return "%s is not a mercurial checkout." % gecko_path + return apikey, gecko_path + + +class TestAIConflictResolutionLive(unittest.TestCase): + def setUp(self): + requirements = _resolve_requirements() + if isinstance(requirements, str): + self.skipTest(requirements) + self.apikey, self.gecko_path = requirements + + # The checkout must be pristine so we don't clobber uncommitted work. + status = hg(["status"], self.gecko_path) + self.assertFalse( + status.stdout.strip(), + "The gecko checkout at %s is not clean:\n%s\nStart from a clean checkout." % ( + self.gecko_path, status.stdout)) + + self.original_rev = _working_parent_node(self.gecko_path) + print("Clean gecko checkout at %s (rev %s)." % (self.gecko_path, self.original_rev[:12])) + + # Register the restore BEFORE we mutate anything, so it runs even if the + # scenario setup below (or the test) raises partway through. + self.addCleanup(self._restore) + + print("Setting up the nestegg conflict scenario (and committing it)...") + _setup_nestegg_scenario(self.gecko_path) + + def _restore(self): + restore_checkout(self.gecko_path, self.original_rev) + + def _resolve_with_real_ai(self): + from components.commandprovider import CommandProvider + from components.logging import SimpleLogger + from components.aiprovider import AIProvider + + logger = SimpleLogger({"local": True, "level": 5}) + command_provider = CommandProvider({}) + command_provider.update_config({"LoggingProvider": logger}) + ai_config = {"apikey": self.apikey} + # Capture the prompt/system prompt/stdout/stderr of each claude invocation + # for debugging (the CLI buffers its JSON until it finishes, so there is + # nothing to watch mid-run -- inspect this afterwards). Honor + # $DEBUG_OUTPUT_DIR if set; otherwise make a fresh /tmp dir that we + # deliberately leave behind so the output survives the run. + debug_dir = os.environ.get("DEBUG_OUTPUT_DIR") + if debug_dir: + debug_dir = os.path.abspath(debug_dir) + else: + debug_dir = tempfile.mkdtemp(prefix="updatebot-ai-debug-", dir="/tmp") + ai_config["debug-output-dir"] = debug_dir + print("AI debug output will be written to %s" % debug_dir) + ai = AIProvider(ai_config) + ai.update_config({"CommandProvider": command_provider, "LoggingProvider": logger}) + + return ai.resolve_patch_conflicts( + MOZ_YAML, + "Bug 0 - update nestegg local patches to apply cleanly (test)", + cwd=self.gecko_path) + + def test_resolve_nestegg_patch_conflict(self): + print("Running the live AI conflict resolution (this calls the real claude CLI)...") + result = self._resolve_with_real_ai() + + self.assertIsNotNone(result, "resolve_patch_conflicts produced no result file.") + outcome = result.get("outcome") + print("AI outcome: %s" % outcome) + print("AI details:\n - " + "\n - ".join(result.get("details", []))) + self.assertIn(outcome, ("trivial success", "uncertain success"), + "AI reported outcome %r; expected a success." % outcome) + + # The patch should now apply cleanly (or have been removed as upstreamed). + reapply = subprocess.run( + ["./mach", "vendor", "--patch-mode", "only", MOZ_YAML], + cwd=self.gecko_path, capture_output=True, text=True) + self.assertEqual( + reapply.returncode, 0, + "After AI resolution, `./mach vendor --patch-mode only` still failed:\n%s" % reapply.stderr) + + print("PASS: the AI resolved the nestegg patch conflict (%s)." % outcome) + + +if __name__ == "__main__": + # Allow running this file directly, like the other test modules + # (e.g. `python tests/local-tests/ai_conflict_resolution.py`). Put the repo + # root on sys.path so the lazy `components.*` / `localconfig` imports resolve + # regardless of the current working directory. + sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))) + unittest.main(verbosity=2) diff --git a/tests/local-tests/full_vendor_run_nestegg.py b/tests/local-tests/full_vendor_run_nestegg.py new file mode 100755 index 00000000..fc0bdd2b --- /dev/null +++ b/tests/local-tests/full_vendor_run_nestegg.py @@ -0,0 +1,261 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +""" +Local, live, END-TO-END test: the whole updatebot vendoring pipeline for a +contrived nestegg update, using real dev credentials. + +This is NOT run by test.py / CI. It runs the real `Updatebot(...).run()` with a +`nestegg` library filter, which -- against the real dev backends -- will: + * detect that nestegg is out of date and `./mach vendor` the new revision, + * file a real bug on the dev Bugzilla, + * hit a local-patch conflict and resolve it with the real `claude` CLI, + * push a real Try run (`./mach try auto --push-to-vcs`), + * submit real revision(s) to the dev Phabricator (`arc diff`), + * and record job/try/phab rows in the dev database. + +It costs money, pushes to Try, and creates real (dev) bugs/revisions, so it is +opt-in and SKIPS unless everything it needs is present. + +How the scenario is contrived +----------------------------- +The trick is to make nestegg look out of date *and* guarantee the local patch +conflicts once it is upgraded: + + 1. Downgrade nestegg to NESTEGG_OLD_REV (the parent of the upstream commit that + refactored ne_read_uint) via `./mach vendor --revision ... --patch-mode + none`. That rewrites moz.yaml's revision *and* the vendored source, so the + later re-vendor is a real (non-spurious) change. + 2. Add the conflicting local patch. Its context is the pre-refactor guard + `if (length == 0 || length > 8)`, which exists at NESTEGG_OLD_REV -- so it + applies cleanly now, but will fail once updatebot upgrades to the refactored + tip (`if (length > 8)`), triggering the AI conflict-resolution path. + 3. Commit it. updatebot's `check_for_update` then sees the tip as a new version + and runs the full pipeline. + +Requirements (SKIPS if any are missing): + - localconfig.py with General.env == 'dev' and Database + Bugzilla blocks + - an AI api key: localconfig['AI']['apikey'] or $ANTHROPIC_API_KEY + - a gecko checkout: localconfig['General']['gecko-path'] or $LIVE_GECKO_PATH + - the `claude` and `arc` CLIs on $PATH + - (implicitly) Try push access for `./mach try --push-to-vcs` + +Teardown restores the gecko checkout only; the filed bug, Phabricator revisions, +and dev DB rows are deliberately LEFT for inspection. +""" + +import os +import sys +import copy +import shutil +import subprocess +import tempfile +import unittest + +# Sibling helpers from the AI-only live test (this directory is on sys.path via +# test_local.py, or via the __main__ block below). We reuse the nestegg fixture, +# the moz.yaml patch-insertion, the hg helper, the working-parent lookup, and the +# checkout-restore logic rather than duplicating them. +from ai_conflict_resolution import ( + LIBRARY_DIR, MOZ_YAML, PATCH_REL, PATCH_FIXTURE, + _add_patch_to_moz_yaml, hg, _working_parent_node, restore_checkout, +) + +# The upstream nestegg revision to downgrade to: the parent of +# 1c9936b1... ("Handle zero-length uint... Bug 2045549"), which refactored the +# ne_read_uint length guard. At this revision the guard is still +# `if (length == 0 || length > 8)`, matching the conflicting patch's context. +NESTEGG_OLD_REV = "405fdae0802b9d6aa0c7937dbc660da197164980" + +SCENARIO_COMMIT_MESSAGE = "TEST SCENARIO (do not land): downgrade nestegg + add conflicting local patch" + +# A local Arcanist install that may not be on PATH. If present, we prepend it so +# both the `arc` requirement check and updatebot's real `arc` subprocesses find +# it (and pick up this specific build over any other). +ARC_BIN_DIR = "/home/tom/packages/arcanist/bin" + + +def _ensure_arc_on_path(): + if os.path.isdir(ARC_BIN_DIR) and ARC_BIN_DIR not in os.environ.get("PATH", "").split(os.pathsep): + os.environ["PATH"] = ARC_BIN_DIR + os.pathsep + os.environ.get("PATH", "") + + +def _resolve_requirements(): + """Return (apikey, gecko_path, localconfig), or a string describing what's missing.""" + _ensure_arc_on_path() + try: + from localconfig import localconfig + except ImportError: + return ("No localconfig.py found. Copy localconfig.py.example to " + "localconfig.py and fill in your dev credentials.") + + # Safety: this test files bugs and submits patches. Refuse to run it against + # anything but an explicit dev environment. + if localconfig.get("General", {}).get("env") != "dev": + return "localconfig['General']['env'] must be 'dev' to run this test (refusing to touch prod)." + + apikey = os.environ.get("ANTHROPIC_API_KEY") or localconfig.get("AI", {}).get("apikey") + gecko_path = os.environ.get("LIVE_GECKO_PATH") or localconfig.get("General", {}).get("gecko-path") + + missing = [] + if not apikey: + missing.append("an AI api key (localconfig['AI']['apikey'] or $ANTHROPIC_API_KEY)") + if not gecko_path: + missing.append("a gecko checkout (localconfig['General']['gecko-path'] or $LIVE_GECKO_PATH)") + if not localconfig.get("Database"): + missing.append("a Database config block (dev DB) in localconfig") + if not localconfig.get("Bugzilla", {}).get("apikey"): + missing.append("a Bugzilla apikey in localconfig['Bugzilla']") + if not shutil.which("claude"): + missing.append("the 'claude' CLI on $PATH") + if not shutil.which("arc"): + missing.append("the 'arc' CLI on $PATH (for Phabricator submission)") + if missing: + return "Missing required configuration:\n - " + "\n - ".join(missing) + + gecko_path = os.path.abspath(gecko_path) + if not os.path.isdir(os.path.join(gecko_path, ".hg")): + return "%s is not a mercurial checkout." % gecko_path + return apikey, gecko_path, localconfig + + +def _build_config(gecko_path, apikey, localconfig): + config = copy.deepcopy(localconfig) + config["General"]["env"] = "dev" + config["General"]["gecko-path"] = gecko_path + # Updatebot._validate requires an hg.mozilla.org repo; the dev localconfig may + # point at a GitHub mirror. The value is only used for validation / a short + # name -- env == 'dev' is what actually routes Bugzilla + Phabricator at their + # dev instances -- so override it with the holly project repo. + config["General"]["repo"] = "https://hg.mozilla.org/projects/holly" + config.setdefault("AI", {})["apikey"] = apikey + + # Capture each claude invocation for debugging (honor $DEBUG_OUTPUT_DIR, else + # a fresh /tmp dir we deliberately leave behind). + debug_dir = os.environ.get("DEBUG_OUTPUT_DIR") + debug_dir = os.path.abspath(debug_dir) if debug_dir else tempfile.mkdtemp( + prefix="updatebot-ai-debug-", dir="/tmp") + config["AI"]["debug-output-dir"] = debug_dir + print("AI debug output will be written to %s" % debug_dir) + return config + + +def _setup_downgraded_nestegg_scenario(gecko_path): + # 1. Downgrade nestegg (moz.yaml revision + vendored source) to a pre-refactor + # revision, so check_for_update sees the tip as new and the re-vendor is a + # real change. --patch-mode none: there are no local patches yet. + print("Downgrading nestegg to %s via `./mach vendor`..." % NESTEGG_OLD_REV[:12]) + vendor = subprocess.run( + ["./mach", "vendor", MOZ_YAML, "--revision", NESTEGG_OLD_REV, "--patch-mode", "none"], + cwd=gecko_path, capture_output=True, text=True) + if vendor.returncode != 0: + raise AssertionError("Downgrade vendor failed (rc=%d):\n%s\n%s" % ( + vendor.returncode, vendor.stdout, vendor.stderr)) + + # 2. Add the conflicting local patch (applies cleanly at the old revision). + lib = os.path.join(gecko_path, LIBRARY_DIR) + os.makedirs(os.path.join(lib, "patches"), exist_ok=True) + with open(PATCH_FIXTURE) as src: + patch_text = src.read() + with open(os.path.join(lib, PATCH_REL), "w") as dst: + dst.write(patch_text) + _add_patch_to_moz_yaml(os.path.join(gecko_path, MOZ_YAML)) + + # 3. Commit everything (source downgrade + moz.yaml + the new patch file). + # --addremove so any files the downgrade added/removed are picked up. + commit = hg(["commit", "--addremove", "-m", SCENARIO_COMMIT_MESSAGE], gecko_path) + if commit.returncode != 0: + raise AssertionError("Committing the scenario failed (rc=%d):\n%s\n%s" % ( + commit.returncode, commit.stdout, commit.stderr)) + + +class TestFullVendorRunNesteggLive(unittest.TestCase): + def setUp(self): + requirements = _resolve_requirements() + if isinstance(requirements, str): + self.skipTest(requirements) + self.apikey, self.gecko_path, self.localconfig = requirements + + # The checkout must be pristine so we don't clobber uncommitted work. + status = hg(["status"], self.gecko_path) + self.assertFalse( + status.stdout.strip(), + "The gecko checkout at %s is not clean:\n%s\nStart from a clean checkout." % ( + self.gecko_path, status.stdout)) + + self.original_rev = _working_parent_node(self.gecko_path) + print("Clean gecko checkout at %s (rev %s)." % (self.gecko_path, self.original_rev[:12])) + + # updatebot chdir's into the gecko path and doesn't chdir back; restore + # both the checkout and our cwd. Registered BEFORE any mutation so they + # run even if scenario setup raises partway through. + self._original_cwd = os.getcwd() + self.addCleanup(lambda: os.chdir(self._original_cwd)) + self.addCleanup(lambda: restore_checkout(self.gecko_path, self.original_rev)) + + print("Setting up the downgraded-nestegg conflict scenario (and committing it)...") + _setup_downgraded_nestegg_scenario(self.gecko_path) + + def _nestegg_library(self, updatebot): + libraries = updatebot.libraryProvider.get_libraries(self.gecko_path) + matches = [lib for lib in libraries if "nestegg" in lib.name] + self.assertTrue(matches, "No nestegg library found in %s" % self.gecko_path) + return matches[0] + + def test_full_vendor_run(self): + from automation import Updatebot + from components.dbmodels import JOBTYPE, JOBSTATUS, JOBOUTCOME + + config = _build_config(self.gecko_path, self.apikey, self.localconfig) + + print("Running the REAL updatebot for nestegg (files a dev bug, pushes Try, " + "submits to dev Phabricator)...") + updatebot = Updatebot(config) + updatebot.run(library_filter="nestegg") + + # Read back what the run recorded. run() catches per-library exceptions + # and logs them rather than raising, so we inspect the DB to judge it. + library = self._nestegg_library(updatebot) + jobs = updatebot.dbProvider.get_all_jobs_for_library(library, JOBTYPE.VENDORING) + self.assertTrue(jobs, "updatebot recorded no vendoring job for nestegg -- did it " + "detect an update? (check_for_update / updatebot_is_enabled)") + + job = max(jobs, key=lambda j: j.id) + bug_url = "https://bugzilla-dev.allizom.org/show_bug.cgi?id=%s" % job.bugzilla_id + print("\n=== Most recent nestegg vendoring job ===") + print(" job id: %s" % job.id) + print(" version: %s" % job.version) + print(" status: %s" % JOBSTATUS(job.status).name) + print(" outcome: %s" % JOBOUTCOME(job.outcome).name) + print(" bug: %s" % (bug_url if job.bugzilla_id else "(none filed)")) + for p in (job.phab_revisions or []): + print(" phabricator: https://phabricator-dev.allizom.org/D%s" % p.revision) + print("=========================================\n") + + self.assertIsNotNone(job.bugzilla_id, "no bug was filed for the nestegg job") + # A fully successful pipeline ends waiting on Try results, with no failure + # outcome recorded. Anything else (COULD_NOT_PATCH / _SUBMIT_TO_TRY / + # _SUBMIT_TO_PHAB) means a stage failed -- the printout above says which. + self.assertEqual( + job.outcome, JOBOUTCOME.PENDING, + "nestegg job finished with a failure outcome (%s); see the job details above." % job.outcome) + self.assertIn( + job.status, + (JOBSTATUS.AWAITING_INITIAL_PLATFORM_TRY_RESULTS, JOBSTATUS.AWAITING_SECOND_PLATFORMS_TRY_RESULTS), + "nestegg job did not reach the awaiting-try-results state; see the job details above.") + + print("PASS: updatebot filed a bug, resolved the patch conflict, pushed Try, and " + "submitted to Phabricator for nestegg (job %s)." % job.id) + + +if __name__ == "__main__": + # Allow running this file directly, like the other test modules. Put the repo + # root AND this directory on sys.path so both `components.*`/`automation` and + # the sibling `ai_conflict_resolution` import resolve regardless of cwd. + here = os.path.dirname(os.path.abspath(__file__)) + sys.path.insert(0, os.path.abspath(os.path.join(here, "..", ".."))) + sys.path.insert(0, here) + unittest.main(verbosity=2) diff --git a/tests/local-tests/nestegg_conflict.patch b/tests/local-tests/nestegg_conflict.patch new file mode 100644 index 00000000..e7439a77 --- /dev/null +++ b/tests/local-tests/nestegg_conflict.patch @@ -0,0 +1,18 @@ +Bug 2050000 - Note the RFC 8794 length limit in ne_read_uint. r?kinetik + +A local Mozilla patch that documents the EBML integer length limit +enforced in ne_read_uint. + +diff --git a/src/nestegg.c b/src/nestegg.c +--- a/src/nestegg.c ++++ b/src/nestegg.c +@@ -1026,7 +1026,8 @@ ne_read_uint(ne_io * io, uint64_t * val, uint64_t length) + unsigned char b; + int r; + +- if (length == 0 || length > 8) ++ /* Mozilla: EBML unsigned integers are 1..8 bytes wide (RFC 8794 s7.1). */ ++ if (length == 0 || length > 8) + return -1; + r = ne_io_read(io, &b, 1); + if (r != 1) diff --git a/tests/mock_aiprovider.py b/tests/mock_aiprovider.py new file mode 100644 index 00000000..e5033aa1 --- /dev/null +++ b/tests/mock_aiprovider.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python3 + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import sys +sys.path.append(".") +sys.path.append("..") + +from components.providerbase import BaseProvider, INeedsLoggingProvider +from components.logging import LogLevel + + +class MockAIProvider(BaseProvider, INeedsLoggingProvider): + """ + A stand-in for AIProvider used by the functionality tests. It never shells + out to a real AI CLI; instead resolve_patch_conflicts returns a result + driven by the provider's config: + + 'ai_outcome': the 'outcome' value to report ('trivial success', + 'uncertain success', 'failure'), or None to simulate the + AI producing no parseable result (resolve returns None). + 'ai_details': the list of detail strings to report. + + It also records how many times it was asked to resolve conflicts so tests + can assert whether the AI path was taken. + """ + + def __init__(self, config): + self.ai_outcome = config.get('ai_outcome', 'failure') + self.ai_details = config.get('ai_details', []) + self.resolve_call_count = 0 + + def resolve_patch_conflicts(self, moz_yaml_path, commit_message, cwd=None, + library_name=None, job_id=None): + self.resolve_call_count += 1 + self.logger.log("MockAIProvider.resolve_patch_conflicts called for %s (outcome=%s)" % ( + moz_yaml_path, self.ai_outcome), level=LogLevel.Info) + if self.ai_outcome is None: + return None + return {"outcome": self.ai_outcome, "details": self.ai_details} diff --git a/tests/mock_commandprovider.py b/tests/mock_commandprovider.py index c4b6f3cc..9d06f3ce 100644 --- a/tests/mock_commandprovider.py +++ b/tests/mock_commandprovider.py @@ -30,7 +30,7 @@ def __init__(self, config): if 'real_runner' in config: self.real_runner = config['real_runner'] - def run(self, args, shell=False, clean_return=True): + def run(self, args, shell=False, clean_return=True, cwd=None, stdin_path=None, timeout=60 * 20, env=None): argument_string = args if isinstance(args, list): argument_string = " ".join(args) diff --git a/tests/run_command.py b/tests/run_command.py index ebc4ea6e..b4246c2b 100755 --- a/tests/run_command.py +++ b/tests/run_command.py @@ -4,7 +4,9 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +import os import sys +import tempfile import unittest sys.path.append(".") @@ -14,12 +16,40 @@ class TestCommandRunner(unittest.TestCase): - def testCommand(self): + def _runner(self): runner = CommandProvider({}) runner.update_config(SimpleLoggerConfig) - ret = runner.run(["echo", "Test"]) + return runner + + def testCommand(self): + ret = self._runner().run(["echo", "Test"]) self.assertEqual(ret.returncode, 0, "Did not run the command successfully") + def testEnvIsMergedOverEnviron(self): + # A provided env var reaches the child, layered on top of the inherited environment. + ret = self._runner().run(["bash", "-c", "echo $UPDATEBOT_TEST_VAR"], + env={"UPDATEBOT_TEST_VAR": "hello"}) + self.assertEqual(ret.returncode, 0) + self.assertEqual(ret.stdout.decode().strip(), "hello") + + def testCwd(self): + with tempfile.TemporaryDirectory() as d: + ret = self._runner().run(["pwd"], cwd=d) + self.assertEqual(ret.returncode, 0) + self.assertEqual(os.path.realpath(ret.stdout.decode().strip()), os.path.realpath(d)) + + def testStdinPath(self): + # The contents of stdin_path are fed to the process's standard input. + with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f: + f.write("piped-in-content") + stdin_path = f.name + try: + ret = self._runner().run(["cat"], stdin_path=stdin_path) + self.assertEqual(ret.returncode, 0) + self.assertEqual(ret.stdout.decode(), "piped-in-content") + finally: + os.remove(stdin_path) + if __name__ == '__main__': unittest.main(verbosity=0)