Handle pull requests

1 files changed, 605 insertions, 30 deletions

diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 6816e5a..ae736ca 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -340,9 +340,46 @@ namespace brep
     }
     else if (event == "pull_request")
     {
-      // @@ TODO
+      gh_pull_request_event pr;
+      try
+      {
+        json::parser p (body.data (), body.size (), "pull_request event");
+
+        pr = gh_pull_request_event (p);
+      }
+      catch (const json::invalid_json_input& e)
+      {
+        string m ("malformed JSON in " + e.name + " request body");
+
+        error << m << ", line: " << e.line << ", column: " << e.column
+              << ", byte offset: " << e.position << ", error: " << e;
 
-      throw invalid_request (501, "pull request events not implemented yet");
+        throw invalid_request (400, move (m));
+      }
+
+      if (pr.action == "opened" || pr.action == "synchronize")
+      {
+        // opened
+        //   A pull request was opened.
+        //
+        // synchronize
+        //   A pull request's head branch was updated from the base branch or
+        //   new commits were pushed to the head branch. (Note that there is
+        //   no equivalent event for the base branch. That case gets handled
+        //   in handle_check_suite_request() instead.)
+        //
+        // Note that both cases are handled the same: we start a new CI
+        // request which will be reported on the new commit id.
+        //
+        return handle_pull_request (move (pr), warning_success);
+      }
+      else
+      {
+        // Ignore the remaining actions by sending a 200 response with empty
+        // body.
+        //
+        return true;
+      }
     }
     else
     {
@@ -410,6 +447,567 @@ namespace brep
     return true;
   }
 
+  // High-level description of pull request (PR) handling
+  //
+  // - Some GitHub pull request terminology:
+  //
+  //   - Fork and pull model: Pull requests are created in a forked
+  //     repository. Thus the head and base repositories are different.
+  //
+  //   - Shared repository model: The pull request head and base branches are
+  //     in the same repository. For example, from a feature branch onto
+  //     master.
+  //
+  // - CI the merge commit but add check runs to the pull request head commit
+  //
+  //   Most of the major CI integrations build the merge commit instead of the
+  //   PR head commit.
+  //
+  //   Adding the check runs to the PR head commit is recommended by the
+  //   following blog posts by a GitHub employee who is one of the best
+  //   sources on these topics:
+  //   https://www.kenmuse.com/blog/shared-commits-and-github-checks/ and
+  //   https://www.kenmuse.com/blog/creating-github-checks/.
+  //
+  //   Do not add any check runs to the merge commit because:
+  //
+  //   - The PR head commit is the only commit that can be relied upon to
+  //     exist throughout the PR's lifetime. The merge commit, on the other
+  //     hand, can change during the PR process. When that happens the PR will
+  //     look for check runs on the new merge commit, effectively discarding
+  //     the ones we had before.
+  //
+  //   - Although some of the GitHub documentation makes it sound like they
+  //     expect check runs to be added to both the PR head commit and the
+  //     merge commit, the PR UI does not react to the merge commit's check
+  //     runs consistently. It actually seems to be quite broken.
+  //
+  //     The only thing it seems to do reliably is blocking the PR merge if
+  //     the merge commit's check runs are not successful (i.e, overriding the
+  //     PR head commit's check runs). But the UI looks quite messed up
+  //     generally in this state.
+  //
+  //   Note that, in the case of a PR from a forked repository (the so-called
+  //   "fork and pull" model), GitHub will copy the PR head commit from the
+  //   head repository (the forked one) into the base repository. So the check
+  //   runs must always be added to the base repository, whether the PR is
+  //   from within the same repository or from a forked repository. The merge
+  //   and head commits will be at refs/pull/<PR-number>/{merge,head}.
+  //
+  // - New commits are added to PR head branch
+  //
+  //   @@ TODO In this case we will end up creating two sets of check runs on
+  //      the same commit (pull_request.head.sha and
+  //      check_suite.head_sha). It's not obvious which to prefer but I'm
+  //      thinking the pull request is more important because in most
+  //      development models it represents something that is more likely to
+  //      end up in an important branch (as opposed to the head of a feature
+  //      branch).
+  //
+  //   Possible solution: ignore all check_suites with non-empty
+  //   pull_requests[].
+  //
+  //   => check_suite(requested, PR_head) [only in shared repo model]
+  //
+  //      Note: check_suite.pull_requests[] will contain all PRs with this
+  //      branch as head.
+  //
+  //      Note: check_suite.pull_requests[i].head.sha will be the new, updated
+  //      PR head sha.
+  //
+  //   => pull_request(synchronize)
+  //
+  //   Note: The check_suite and pull_request can arrive in any order.
+  //
+  // - New commits are added to PR base branch
+  //
+  //   Note: In this case pull_request.base.sha does not change, but the merge
+  //   commit will be updated to include the new commits to the base branch.
+  //
+  // - @@ TODO? PR base branch changed (to a different branch)
+  //
+  //   => pull_request(edited)
+  //
+  // - PR closed @@ TODO
+  //
+  //   => pull_request(closed)
+  //
+  //   Cancel CI?
+  //
+  // - PR merged @@ TODO
+  //
+  //   => pull_request(merged)
+  //
+  //   => check_suite(PR_base)
+  //
+  //   Probably wouldn't want to CI the base again because the PR CI would've
+  //   done the equivalent already.
+  //
+  bool ci_github::
+  handle_pull_request (gh_pull_request_event pr, bool warning_success)
+  {
+    HANDLER_DIAG;
+
+    l3 ([&]{trace << "pull_request event { " << pr << " }";});
+
+    // While we don't need the installation access token in this request,
+    // let's obtain it to flush out any permission issues early. Also, it is
+    // valid for an hour so we will most likely make use of it.
+    //
+    optional<string> jwt (generate_jwt (trace, error));
+    if (!jwt)
+      throw server_error ();
+
+    optional<gh_installation_access_token> iat (
+      obtain_installation_access_token (pr.installation.id,
+                                        move (*jwt),
+                                        error));
+    if (!iat)
+      throw server_error ();
+
+    l3 ([&]{trace << "installation_access_token { " << *iat << " }";});
+
+    string sd (service_data (warning_success,
+                             move (iat->token),
+                             iat->expires_at,
+                             pr.installation.id,
+                             move (pr.repository.node_id),
+                             pr.pull_request.head_sha,
+                             pr.repository.clone_url,
+                             pr.pull_request.number)
+                 .json ());
+
+    // Create unloaded CI request. After this call we will start getting the
+    // build_unloaded() notifications until (1) we load the request, (2) we
+    // cancel it, or (3) it gets archived after some timeout.
+    //
+    // Note: use no delay since we need to (re)create the synthetic merge
+    // check run as soon as possible.
+    //
+    optional<string> tid (
+      create (error, warn, &trace,
+              *build_db_,
+              tenant_service (move (pr.pull_request.node_id),
+                              "ci-github",
+                              move (sd)),
+              chrono::seconds (30) /* interval */,
+              chrono::seconds (0)  /* delay */));
+
+    if (!tid)
+      fail << "unable to create unloaded CI request";
+
+    return true;
+  }
+
+  // Return the colored circle corresponding to a result_status.
+  //
+  static string
+  circle (result_status rs)
+  {
+    switch (rs)
+    {
+    case result_status::success:  return "\U0001F7E2"; // Green circle.
+    case result_status::warning:  return "\U0001F7E0"; // Orange circle.
+    case result_status::error:
+    case result_status::abort:
+    case result_status::abnormal: return "\U0001F534"; // Red circle.
+
+      // Valid values we should never encounter.
+      //
+    case result_status::skip:
+    case result_status::interrupt:
+      throw invalid_argument ("unexpected result_status value: " +
+                              to_string (rs));
+    }
+
+    return ""; // Should never reach.
+  }
+
+  // Let's capitalize the synthetic check run names to make them easier to
+  // distinguish from the regular ones.
+  //
+  static string merge_check_run_name ("MERGE-COMMIT");
+  static string conclusion_check_run_name ("CONCLUSION");
+
+  function<optional<string> (const tenant_service&)> ci_github::
+  build_unloaded (tenant_service&& ts,
+                  const diag_epilogue& log_writer) const noexcept
+  {
+    NOTIFICATION_DIAG (log_writer);
+
+    service_data sd;
+    try
+    {
+      sd = service_data (*ts.data);
+    }
+    catch (const invalid_argument& e)
+    {
+      error << "failed to parse service data: " << e;
+      return nullptr;
+    }
+
+    // Get a new installation access token if the current one has expired.
+    //
+    const gh_installation_access_token* iat (nullptr);
+    optional<gh_installation_access_token> new_iat;
+
+    if (system_clock::now () > sd.installation_access.expires_at)
+    {
+      if (optional<string> jwt = generate_jwt (trace, error))
+      {
+        new_iat = obtain_installation_access_token (sd.installation_id,
+                                                    move (*jwt),
+                                                    error);
+        if (new_iat)
+          iat = &*new_iat;
+      }
+    }
+    else
+      iat = &sd.installation_access;
+
+    if (iat == nullptr)
+      return nullptr; // Try again on the next call.
+
+    auto make_iat_updater = [&new_iat, &error] ()
+    {
+      function<optional<string> (const tenant_service&)> r;
+
+      if (new_iat)
+      {
+        r = [&error,
+             iat = move (new_iat)] (const tenant_service& ts)
+          -> optional<string>
+        {
+          // NOTE: this lambda may be called repeatedly (e.g., due to
+          // transaction being aborted) and so should not move out of its
+          // captures.
+
+          service_data sd;
+          try
+          {
+            sd = service_data (*ts.data);
+          }
+          catch (const invalid_argument& e)
+          {
+            error << "failed to parse service data: " << e;
+            return nullopt;
+          }
+
+          sd.installation_access = *iat;
+
+          return sd.json ();
+        };
+      }
+
+      return r;
+    };
+
+    // Create a synthetic check run with an in-progress state. Return the
+    // check run on success or nullopt on failure.
+    //
+    auto create_synthetic_cr = [iat,
+                                &sd,
+                                &error] (string name) -> optional<check_run>
+    {
+      check_run cr;
+      cr.name = move (name);
+
+      if (gq_create_check_run (error,
+                               cr,
+                               iat->token,
+                               sd.repository_node_id,
+                               sd.report_sha,
+                               nullopt /* details_url */,
+                               build_state::building))
+      {
+        return cr;
+      }
+      else
+        return nullopt;
+    };
+
+    // Update a synthetic check run with success or failure. Return the check
+    // run on success or nullopt on failure.
+    //
+    auto update_synthetic_cr = [iat,
+                                &sd,
+                                &error] (const string& node_id,
+                                         const string& name,
+                                         result_status rs,
+                                         string summary) -> optional<check_run>
+    {
+      assert (!node_id.empty ());
+
+      optional<gq_built_result> br (
+        gq_built_result (gh_to_conclusion (rs, sd.warning_success),
+                         circle (rs) + ' ' + ucase (to_string (rs)),
+                         move (summary)));
+
+      check_run cr;
+      cr.name = name; // For display purposes only.
+
+      if (gq_update_check_run (error,
+                               cr,
+                               iat->token,
+                               sd.repository_node_id,
+                               node_id,
+                               nullopt /* details_url */,
+                               build_state::built,
+                               move (br)))
+      {
+        return cr;
+      }
+      else
+        return nullopt;
+    };
+
+    // Synthetic merge check run node ID. Empty until created on the first
+    // call or retrieved from service data on subsequent calls.
+    //
+    string merge_node_id;
+
+    // True if this is the first call (or the merge commit couldn't be created
+    // on the first call, in which case we just re-try by treating it as a
+    // first call).
+    //
+    bool first (!sd.merge_node_id);
+
+    // If this is the first call, (re)create the synthetic merge check run as
+    // soon as possible to make sure the previous check suite, if any, is no
+    // longer completed.
+    //
+    // Note that there is still a window between receipt of the pull_request
+    // event and the first bot/worker asking for a task, which could be
+    // substantial. We could probably (also) try to (re)create the merge
+    // checkrun in the webhook. @@ Maybe/later.
+    //
+    if (first)
+    {
+      if (auto cr = create_synthetic_cr (merge_check_run_name))
+      {
+        l3 ([&]{trace << "created check_run { " << *cr << " }";});
+
+        merge_node_id = move (*cr->node_id);
+      }
+      else
+        return make_iat_updater (); // Try again on the next call.
+    }
+    else
+      merge_node_id = *sd.merge_node_id;
+
+    // Start/check PR mergeability.
+    //
+    optional<string> mc (
+      gq_pull_request_mergeable (error, iat->token, ts.id)); // Merge commit.
+
+    if (!mc || mc->empty ())
+    {
+      if (!mc) // No merge commit yet.
+      {
+        // If this is a subsequent notification and there is no merge commit,
+        // then there is nothing to do.
+        //
+        if (!first)
+          return make_iat_updater ();
+
+        // Fall through to update service data.
+      }
+      else // Not mergeable.
+      {
+        // If the commit is not mergeable, cancel the CI request and fail the
+        // merge check run.
+        //
+        // Note that it feels like in this case we don't really need to create a
+        // failed synthetic conclusion check run since the PR cannot be merged
+        // anyway.
+
+        if (auto cr = update_synthetic_cr (
+              merge_node_id,
+              merge_check_run_name,
+              result_status::error,
+              "GitHub is unable to create test merge commit"))
+        {
+          l3 ([&]{trace << "updated check_run { " << *cr << " }";});
+
+          // Cancel the CI request.
+          //
+          if (!cancel (error, warn, &trace, *build_db_, ts.type, ts.id))
+          {
+            // Nothing we can do but also highly unlikely.
+            //
+            error << "unable to cancel CI request: "
+                  << "no tenant for service type/ID "
+                  << ts.type << '/' << ts.id;
+          }
+
+          return nullptr; // No need to update service data in this case.
+        }
+        else
+        {
+          // Don't cancel the CI request if the merge check run update failed
+          // so that we can try again on the next call.
+
+          if (!first)
+            return make_iat_updater ();
+
+          // Fall through to update service data.
+        }
+      }
+
+      // This is a first notification, so record the merge check run in the
+      // service data.
+      //
+      return [&error,
+              iat = move (new_iat),
+              mni = move (merge_node_id)] (const tenant_service& ts)
+        -> optional<string>
+      {
+        // NOTE: this lambda may be called repeatedly (e.g., due to
+        // transaction being aborted) and so should not move out of its
+        // captures.
+
+        service_data sd;
+        try
+        {
+          sd = service_data (*ts.data);
+        }
+        catch (const invalid_argument& e)
+        {
+          error << "failed to parse service data: " << e;
+          return nullopt;
+        }
+
+        if (iat)
+          sd.installation_access = *iat;
+
+        sd.merge_node_id = mni;
+
+        return sd.json ();
+      };
+    }
+
+    // If we are here, then it means we have a merge commit that we can load.
+    //
+    // Note that this can still be the first call (first=true).
+    //
+
+    // As a first step, (re)create the synthetic conclusion check run and then
+    // change the merge check run state to success. Do it in this order so
+    // that the check suite does not become completed.
+
+    // Synthetic conclusion check run node ID. Empty until created on the
+    // "second" call or retrieved from service data on subsequent calls.
+    //
+    string conclusion_node_id;
+
+    // True if this is the first call after the merge commit became available,
+    // which we will call the "second" call (or we couldn't create the
+    // conclusion check run on the first such call, in which case we just
+    // re-try by treating it as a "second" call).
+    //
+    bool second (!sd.conclusion_node_id);
+
+    if (second)
+    {
+      if (auto cr = create_synthetic_cr (conclusion_check_run_name))
+      {
+        l3 ([&]{trace << "created check_run { " << *cr << " }";});
+
+        conclusion_node_id = move (*cr->node_id);
+      }
+    }
+    else
+      conclusion_node_id = *sd.conclusion_node_id;
+
+    if (!conclusion_node_id.empty ()) // Conclusion check run was created.
+    {
+      // Update merge check run to successful.
+      //
+      if (auto cr = update_synthetic_cr (merge_node_id,
+                                         merge_check_run_name,
+                                         result_status::success,
+                                         "GitHub created test merge commit"))
+      {
+        l3 ([&]{trace << "updated check_run { " << *cr << " }";});
+
+        // Load the CI request.
+        //
+        // Example repository URL fragment:
+        //
+        //   #pull/28/merge@1b6c9a361086ed93e6f1e67189e82d52de91c49b
+        //
+        repository_location rl (*sd.repository_clone_url + "#pull/" +
+                                to_string (*sd.pr_number) + "/merge@" + *mc,
+                                repository_type::git);
+
+        optional<start_result> r (
+          load (error, warn, &trace, *build_db_, move (ts), rl));
+
+        if (!r || r->status != 200)
+        {
+          string msg;
+          msg = "```\n";
+          if (r) msg += r->message;
+          else msg += "Internal service error";
+          msg += "\n```";
+
+          if (auto cr = update_synthetic_cr (conclusion_node_id,
+                                             conclusion_check_run_name,
+                                             result_status::error,
+                                             move (msg)))
+          {
+            l3 ([&]{trace << "updated check_run { " << *cr << " }";});
+          }
+          else
+          {
+            // Nothing really we can do in this case since we will not receive
+            // any further notifications.
+          }
+
+          return nullptr; // No need to update service data in this case.
+        }
+      }
+      else
+      {
+        // Don't load the CI request if the merge check run update failed so
+        // that we can try again on the next call.
+      }
+    }
+
+    return [&error,
+            iat = move (new_iat),
+            mni = (first ? move (merge_node_id) : string ()),
+            cni = (second ? move (conclusion_node_id) : string ())]
+      (const tenant_service& ts) -> optional<string>
+    {
+      // NOTE: this lambda may be called repeatedly (e.g., due to
+      // transaction being aborted) and so should not move out of its
+      // captures.
+
+      service_data sd;
+      try
+      {
+        sd = service_data (*ts.data);
+      }
+      catch (const invalid_argument& e)
+      {
+        error << "failed to parse service data: " << e;
+        return nullopt;
+      }
+
+      if (iat)
+        sd.installation_access = *iat;
+
+      if (!mni.empty ())
+        sd.merge_node_id = mni;
+
+      if (!cni.empty ())
+        sd.conclusion_node_id = cni;
+
+      return sd.json ();
+    };
+  }
+
   // Build state change notifications (see tenant-services.hxx for
   // background). Mapping our state transitions to GitHub pose multiple
   // problems:
@@ -596,7 +1194,7 @@ namespace brep
       if (gq_create_check_runs (error,
                                 crs,
                                 iat->token,
-                                sd.repository_node_id, sd.head_sha,
+                                sd.repository_node_id, sd.report_sha,
                                 build_state::queued))
       {
         for (const check_run& cr: crs)
@@ -873,29 +1471,6 @@ namespace brep
     //
     if (iat != nullptr)
     {
-      // Return the colored circle corresponding to a result_status.
-      //
-      auto circle = [] (result_status rs) -> string
-      {
-        switch (rs)
-        {
-        case result_status::success:  return "\U0001F7E2"; // Green circle.
-        case result_status::warning:  return "\U0001F7E0"; // Orange circle.
-        case result_status::error:
-        case result_status::abort:
-        case result_status::abnormal: return "\U0001F534"; // Red circle.
-
-          // Valid values we should never encounter.
-          //
-        case result_status::skip:
-        case result_status::interrupt:
-          throw invalid_argument ("unexpected result_status value: " +
-                                  to_string (rs));
-        }
-
-        return ""; // Should never reach.
-      };
-
       // Prepare the check run's summary field (the build information in an
       // XHTML table).
       //
@@ -927,9 +1502,9 @@ namespace brep
         // Serialize a result row (colored circle, result text, log URL) for
         // an operation and result_status.
         //
-        auto tr_result = [this, circle, &b] (xml::serializer& s,
-                                             const string& op,
-                                             result_status rs)
+        auto tr_result = [this, &b] (xml::serializer& s,
+                                     const string& op,
+                                     result_status rs)
         {
           // The log URL.
           //
@@ -1030,7 +1605,7 @@ namespace brep
                                  cr,
                                  iat->token,
                                  sd.repository_node_id,
-                                 sd.head_sha,
+                                 sd.report_sha,
                                  details_url (b),
                                  build_state::built,
                                  move (br)))