handle_pull_request(): Create pre-check CI request

3 files changed, 53 insertions, 61 deletions

diff --git a/mod/ci-common.hxx b/mod/ci-common.hxx
index 4bd63c2..c03abf8 100644
--- a/mod/ci-common.hxx
+++ b/mod/ci-common.hxx
@@ -101,6 +101,9 @@ namespace brep
     //
     // Note: should be called out of the database transaction.
     //
+    // @@ TMP Shouldn't the comments mention that if tenant_service.id is an
+    //    empty string then the generated tenant id will be used?
+    //
     enum class duplicate_tenant_mode {fail, ignore, replace, replace_archived};
     enum class duplicate_tenant_result {created, ignored, replaced};
 
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index b03298c..6c0bb72 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -493,8 +493,15 @@ namespace brep
 
     // Create an unloaded CI request.
     //
+    // Note: use no delay since we need to (re)create the synthetic conclusion
+    // check run as soon as possible.
+    //
     // Note that we use the create() API instead of start() since duplicate
-    // management is no available in start().
+    // management is not available in start().
+    //
+    // 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.
     //
     auto pr (create (error,
                      warn,
@@ -524,6 +531,8 @@ namespace brep
 
   // High-level description of pull request (PR) handling
   //
+  // @@ TODO Update these comments.
+  //
   // - Some GitHub pull request terminology:
   //
   //   - Fork and pull model: Pull requests are created in a forked
@@ -681,27 +690,56 @@ namespace brep
 
     l3 ([&]{trace << "installation_access_token { " << *iat << " }";});
 
+    // @@ TMP Regarding pushes to PR head branches ("synchronize"): If a
+    //    remote PR head branch is not behind base, wouldn't we want to cancel
+    //    existing CIs and restart? This is the only event we'll see when the
+    //    user clicks the "update PR head branch with base" button.
+    //
+
+    // @@ TMP We can already determine whether the PR is local or remote so we
+    //    could pass `kind` to the service_data constructor.
+
+    // @@ TODO Serialize the new service_data fields!
+    //
+    // Note that check_sha will be set later, in build_unloaded_pre_check().
+    //
     service_data sd (warning_success,
                      move (iat->token),
                      iat->expires_at,
                      pr.installation.id,
                      move (pr.repository.node_id),
-                     pr.pull_request.head_sha /* report_sha */,
-                     pr.repository.clone_url,
+                     move (pr.pull_request.head_sha) /* report_sha */,
+                     move (pr.repository.clone_url),
                      pr.pull_request.number);
 
-    // Create unloaded CI request. Cancel the existing CI request first if the
-    // head branch has been updated (action is `synchronize`).
+    assert (sd.pre_check);
+
+    // Create an unloaded CI request for the pre-check phase (during which we
+    // wait for the PR's merge commit and behindness to become available).
     //
-    // 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.
+    // Create with an empty service id so that the generated tenant id is used
+    // instead during the pre-check phase (so as not to clash with a proper
+    // service id for this head commit, potentially created in
+    // handle_check_suite()).
     //
-    bool cancel_first (pr.action == "synchronize");
+    tenant_service ts ("", "ci-github", sd.json ());
 
-    if (!create_pull_request_ci (error, warn, trace,
-                                 sd, pr.pull_request.node_id,
-                                 cancel_first))
+    // Note: use no delay since we need to (re)create the synthetic conclusion
+    // check run as soon as possible.
+    //
+    // After this call we will start getting the build_unloaded()
+    // notifications -- which will be routed to build_unloaded_pre_check() --
+    // until we cancel the request or it gets archived after some
+    // timeout. (Note that we never actually load this request, we always
+    // cancel it; see build_unloaded_pre_check() for details.)
+    //
+    if (!create (error,
+                 warn,
+                 &trace,
+                 *build_db_,
+                 move (ts),
+                 chrono::seconds (30) /* interval */,
+                 chrono::seconds (0) /* delay */))
     {
       fail << "pull request " << pr.pull_request.node_id
            << ": unable to create unloaded CI request";
@@ -1881,36 +1919,6 @@ namespace brep
     };
   }
 
-  bool ci_github::
-  create_pull_request_ci (const basic_mark& error,
-                          const basic_mark& warn,
-                          const basic_mark& trace,
-                          const service_data& sd,
-                          const string& nid,
-                          bool cf) const
-  {
-    // Cancel the existing CI request if asked to do so. Ignore failure
-    // because the request may already have been cancelled for other reasons.
-    //
-    if (cf)
-    {
-      if (!cancel (error, warn, &trace, *build_db_, "ci-github", nid))
-        l3 ([&] {trace << "unable to cancel CI for pull request " << nid;});
-    }
-
-    // Create a new unloaded CI request.
-    //
-    tenant_service ts (nid, "ci-github", sd.json ());
-
-    // Note: use no delay since we need to (re)create the synthetic merge
-    // check run as soon as possible.
-    //
-    return create (error, warn, &trace,
-                   *build_db_, move (ts),
-                   chrono::seconds (30) /* interval */,
-                   chrono::seconds (0)  /* delay */).has_value ();
-  }
-
   string ci_github::
   details_url (const build& b) const
   {
diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx
index e919065..8284f7f 100644
--- a/mod/mod-ci-github.hxx
+++ b/mod/mod-ci-github.hxx
@@ -91,25 +91,6 @@ namespace brep
     bool
     handle_pull_request (gh_pull_request_event, bool warning_success);
 
-    // Create an unloaded CI request for a pull request. If `cancel_first` is
-    // true, cancel its existing CI request first.
-    //
-    // Return true if an unloaded CI request was created. Ignore failure to
-    // cancel because the CI request may already have been cancelled for other
-    // reasons.
-    //
-    // 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.
-    //
-    bool
-    create_pull_request_ci (const basic_mark& error,
-                            const basic_mark& warn,
-                            const basic_mark& trace,
-                            const service_data&,
-                            const string& pull_request_node_id,
-                            bool cancel_first) const;
-
     // Build a check run details_url for a build.
     //
     string