Add support for build artifacts upload in agent

8 files changed, 883 insertions, 143 deletions

diff --git a/bbot/agent/agent.cxx b/bbot/agent/agent.cxx
index 6163c14..e56f742 100644
--- a/bbot/agent/agent.cxx
+++ b/bbot/agent/agent.cxx
@@ -31,6 +31,7 @@
 #include <system_error> // generic_category()
 
 #include <libbutl/pager.hxx>
+#include <libbutl/base64.hxx>
 #include <libbutl/sha256.hxx>
 #include <libbutl/openssl.hxx>
 #include <libbutl/filesystem.hxx> // dir_iterator, try_rmfile(), readsymlink()
@@ -47,6 +48,7 @@
 
 #include <bbot/agent/tftp.hxx>
 #include <bbot/agent/machine.hxx>
+#include <bbot/agent/http-service.hxx>
 
 using namespace butl;
 using namespace bbot;
@@ -1249,7 +1251,35 @@ catch (const system_error& e)
 //
 struct interrupt {};
 
-static result_manifest
+struct perform_task_result
+{
+  auto_rmdir      work_dir; // <tftp>/build/<toolchain>-<instance>/
+  result_manifest manifest;
+
+  // Uploaded archive, if any (somewhere inside work_dir).
+  //
+  optional<path> upload_archive;
+
+  // Create the special empty result.
+  //
+  perform_task_result () = default;
+
+  // Create task result without build artifacts.
+  //
+  explicit
+  perform_task_result (auto_rmdir&& d, result_manifest&& m)
+      : work_dir (move (d)),
+        manifest (move (m)) {}
+
+  // Create task result with build artifacts.
+  //
+  perform_task_result (auto_rmdir&& d, result_manifest&& m, path&& a)
+      : work_dir (move (d)),
+        manifest (move (m)),
+        upload_archive (move (a)) {}
+};
+
+static perform_task_result
 perform_task (toolchain_lock tl, // Note: assumes ownership.
               machine_lock& ml,
               const dir_path& md,
@@ -1267,6 +1297,11 @@ try
   sigurs1.store (0, std::memory_order_release);
   tl.unlock ();
 
+  const string in_name (tc_name + '-' + to_string (inst));
+  auto_rmdir arm ((dir_path (ops.tftp ()) /= "build") /= in_name);
+
+  try_mkdir_p (arm.path);
+
   result_manifest r {
     tm.name,
     tm.version,
@@ -1276,7 +1311,7 @@ try
     nullopt /* dependency_checksum */};
 
   if (ops.fake_build ())
-    return r;
+    return perform_task_result (move (arm), move (r));
 
   // The overall plan is as follows:
   //
@@ -1295,12 +1330,9 @@ try
 
   // TFTP server mapping (server chroot is --tftp):
   //
-  // GET requests to .../build/<name>-<instance>/get/*
-  // PUT requests to .../build/<name>-<instance>/put/*
+  // GET requests to .../build/<toolchain>-<instance>/get/*
+  // PUT requests to .../build/<toolchain>-<instance>/put/*
   //
-  const string in_name (tc_name + '-' + to_string (inst));
-  auto_rmdir arm ((dir_path (ops.tftp ()) /= "build") /= in_name);
-
   dir_path gd (dir_path (arm.path) /= "get");
   dir_path pd (dir_path (arm.path) /= "put");
 
@@ -1334,43 +1366,11 @@ try
     }
 
     r = parse_manifest<result_manifest> (rf, "result");
-
-    // If archive of build artifacts is present, then just list its content as
-    // a sanity check.
-    //
-    bool err (!r.status);
-    if (!err && file_exists (af))
-    {
-      try
-      {
-        auto_fd null (fdopen_null ());
-
-        // Redirect stdout to stderr if the command is traced and to /dev/null
-        // otherwise.
-        //
-        process_exit pe (
-          process_run_callback (
-            trace,
-            null.get (), // Don't expect to read from stdin.
-            verb >= 3 ? 2 : null.get (),
-            2,
-            "tar",
-            "-tf", af));
-
-        if (!pe)
-          fail << "tar " << pe;
-      }
-      catch (const process_error& e)
-      {
-        fail << "unable execute tar: " << e;
-      }
-    }
   }
   else
   {
     try_rmfile (rf);
     try_rmfile (af);
-    try_rmdir_r (pd / dir_path ("upload"));
 
     // <name>-<toolchain>-<xxx>
     //
@@ -1418,7 +1418,7 @@ try
               }
             }));
 
-        auto soft_fail = [&trace, &ml, &xp, &m, &r] (const char* msg)
+        auto soft_fail = [&trace, &ml, &xp, &m, &arm, &r] (const char* msg)
         {
           {
             diag_record dr (error);
@@ -1441,7 +1441,7 @@ try
           }
           catch (const failed&) {}
 
-          return r;
+          return perform_task_result (move (arm), move (r));
         };
 
         auto check_machine = [&xp, &m] ()
@@ -1501,7 +1501,7 @@ try
           if (!check_machine ())
           {
             run_btrfs (trace, "subvolume", "delete", xp);
-            return r;
+            return perform_task_result (move (arm), move (r));
           }
         }
 
@@ -1541,7 +1541,7 @@ try
             if (!file_not_empty (rf))
             {
               run_btrfs (trace, "subvolume", "delete", xp);
-              return r;
+              return perform_task_result (move (arm), move (r));
             }
           }
 
@@ -1558,81 +1558,14 @@ try
 
           // Parse the result manifest.
           //
-          optional<result_manifest> rm;
-
           try
           {
-            rm = parse_manifest<result_manifest> (rf, "result", false);
+            r = parse_manifest<result_manifest> (rf, "result", false);
           }
           catch (const failed&)
           {
             r.status = result_status::abnormal; // Soft-fail below.
           }
-
-          // Upload the build artifacts if the result manifest is parsed
-          // successfully, the result status is not an error, and upload.tar
-          // exists.
-          //
-          // Note that while the worker doesn't upload the build artifacts
-          // archives on errors, there can be the case when the error occurred
-          // while uploading the archive and so the partially uploaded file
-          // may exist. Thus, we check if the result status is not an error.
-          //
-          // Note also that we will not bother with interrupting this process
-          // assuming it will be quick (relative to the amount of work that
-          // would be wasted).
-          //
-          bool err (!rm || !rm->status);
-          if (!err && file_exists (af))
-          {
-            // Extract the build artifacts from the archive and upload them to
-            // the controller. On error keep the result status as abort for
-            // transient errors (network failure, etc) and set it to abnormal
-            // otherwise (for subsequent machine suspension and
-            // investigation).
-            //
-            optional<bool> err; // True if the error is transient.
-
-            try
-            {
-              process_exit pe (
-                process_run_callback (
-                  trace,
-                  fdopen_null (), // Don't expect to read from stdin.
-                  2,              // Redirect stdout to stderr.
-                  2,
-                  "tar",
-                  "-xf", af,
-                  "-C", pd));
-
-              if (!pe)
-              {
-                err = false;
-                error << "tar " << pe;
-              }
-            }
-            catch (const process_error& e)
-            {
-              err = false;
-              error << "unable execute tar: " << e;
-            }
-
-            if (!err)
-            {
-              // @@ Upload the extracted artifacts.
-            }
-
-            if (err)
-            {
-              if (!*err)                            // Non-transient?
-                r.status = result_status::abnormal; // Soft-fail below.
-
-              rm = nullopt; // Drop the parsed manifest.
-            }
-          }
-
-          if (rm)
-            r = move (*rm);
         }
         else
         {
@@ -1681,7 +1614,9 @@ try
     r.version = tm.version;
   }
 
-  return r;
+  return (!r.status || !file_exists (af)
+          ? perform_task_result (move (arm), move (r))
+          : perform_task_result (move (arm), move (r), move (af)));
 }
 catch (const system_error& e)
 {
@@ -2206,9 +2141,10 @@ try
           auto t (parse_manifest<task_manifest> (ops.fake_request (), "task"));
 
           tr = task_response_manifest {
-            "fake-session", // Dummy session.
-            nullopt,        // No challenge.
-            string (),      // Empty result URL.
+            "fake-session",        // Dummy session.
+            nullopt,               // No challenge.
+            string (),             // Empty result URL.
+            vector<upload_url> (),
             agent_checksum,
             move (t)};
 
@@ -2381,7 +2317,7 @@ try
     // that we have already received a task, responding with an interrupt
     // feels like the most sensible option).
     //
-    result_manifest r;
+    perform_task_result r;
     bootstrapped_machine* pm (nullptr);
     try
     {
@@ -2592,21 +2528,27 @@ try
     }
     catch (const interrupt&)
     {
-      r = result_manifest {
-        t.name,
-        t.version,
-        result_status::interrupt,
-        operation_results {},
-        nullopt /* worker_checksum */,
-        nullopt /* dependency_checksum */};
+      // Note: no work_dir.
+      //
+      r = perform_task_result (
+            auto_rmdir (),
+            result_manifest {
+              t.name,
+              t.version,
+              result_status::interrupt,
+              operation_results {},
+              nullopt /* worker_checksum */,
+              nullopt /* dependency_checksum */});
     }
 
     if (pm != nullptr && pm->lock.locked ())
       pm->lock.unlock (); // No need to hold the lock any longer.
 
+    result_manifest& rm (r.manifest);
+
     if (ops.dump_result ())
     {
-      serialize_manifest (r, cout, "stdout", "result");
+      serialize_manifest (rm, cout, "stdout", "result");
       return 0;
     }
 
@@ -2642,14 +2584,258 @@ try
       fail << "unable to sign task response challenge: " << e;
     }
 
-    result_status rs (r.status);
+    // Re-package the build artifacts, if present, into the type/instance-
+    // specific archives and upload them to the type-specific URLs, if
+    // provided.
+    //
+    // Note that the initial upload archive content is organized as a bunch of
+    // upload/<type>/<instance>/*, where the second level directories are the
+    // upload types and the third level sub-directories are their instances.
+    // The resulting <instance>.tar archives content (which is what we submit
+    // to the type-specific handler) are organized as <instance>/*.
+    //
+    if (r.upload_archive && !tr.upload_urls.empty ())
+    {
+      const path& ua (*r.upload_archive);
+
+      // Extract the archive content into the parent directory of the archive
+      // file. But first, make sure the resulting directory doesn't exist.
+      //
+      // Note that while we don't assume where inside the working directory
+      // the archive is, we do assume that there is nothing clashing/precious
+      // in the upload/ directory which we are going to cleanup.
+      //
+      dir_path d (ua.directory ());
+
+      const dir_path ud (d / dir_path ("upload"));
+      try_rmdir_r (ud);
+
+      try
+      {
+        process_exit pe (
+          process_run_callback (
+            trace,
+            fdopen_null (), // Don't expect to read from stdin.
+            2,              // Redirect stdout to stderr.
+            2,
+            "tar",
+            "-xf", ua,
+            "-C", d));
+
+        if (!pe)
+          fail << "tar " << pe;
+      }
+      catch (const system_error& e)
+      {
+        // There must be something wrong with the setup or there is no space
+        // left on the disk, thus the failure is fatal.
+        //
+        fail << "unable to extract build artifacts from archive: " << e;
+      }
+
+      try_rmfile (ua); // Let's free up the disk space.
+
+      // To decrease nesting a bit, let's collect the type-specific upload
+      // directories and the corresponding URLs first. This way we can also
+      // create the archive files as the upload/ directory sub-entries without
+      // interfering with iterating over this directory.
+      //
+      vector<pair<dir_path, string>> urls;
+
+      try
+      {
+        for (const dir_entry& te: dir_iterator (ud, dir_iterator::no_follow))
+        {
+          const string& t (te.path ().string ());
+
+          // Can only be a result of the worker malfunction, thus the failure
+          // is fatal.
+          //
+          if (te.type () != entry_type::directory)
+            fail << "unexpected filesystem entry '" << t << "' in " << ud;
+
+          auto i (find_if (tr.upload_urls.begin (), tr.upload_urls.end (),
+                           [&t] (const upload_url& u) {return u.type == t;}));
+
+          if (i == tr.upload_urls.end ())
+            continue;
+
+          urls.emplace_back (ud / path_cast<dir_path> (te.path ()), i->url);
+        }
+      }
+      catch (const system_error& e)
+      {
+        fail << "unable to iterate over " << ud << ": " << e;
+      }
+
+      // Now create archives and upload.
+      //
+      for (const pair<dir_path, string>& p: urls)
+      {
+        const dir_path& td (p.first); // <type>/
+        const string& url (p.second);
+
+        try
+        {
+          for (const dir_entry& ie: dir_iterator (td, dir_iterator::no_follow))
+          {
+            const string& i (ie.path ().string ()); // <instance>
+
+            // Can only be a result of the worker malfunction, thus the
+            // failure is fatal.
+            //
+            if (ie.type () != entry_type::directory)
+              fail << "unexpected filesystem entry '" << i << "' in " << td;
+
+            // Archive the upload instance files and, while at it, calculate
+            // the resulting archive checksum.
+            //
+            sha256 sha;
+            auto_rmfile ari (ud / (i + ".tar"));
+
+            try
+            {
+              // Instruct tar to print the archive to stdout.
+              //
+              fdpipe in_pipe (fdopen_pipe (fdopen_mode::binary));
+
+              process pr (
+                process_start_callback (
+                  trace,
+                  fdopen_null (), // Don't expect to read from stdin.
+                  in_pipe,
+                  2 /* stderr */,
+                  "tar",
+                  "--format", "ustar",
+                  "-c",
+                  "-C", td,
+                  i));
+
+              // Shouldn't throw, unless something is severely damaged.
+              //
+              in_pipe.out.close ();
+
+              ifdstream is (
+                move (in_pipe.in), fdstream_mode::skip, ifdstream::badbit);
+
+              ofdstream os (ari.path, fdopen_mode::binary);
+
+              char buf[8192];
+              while (!eof (is))
+              {
+                is.read (buf, sizeof (buf));
+
+                if (size_t n = static_cast<size_t> (is.gcount ()))
+                {
+                  sha.append (buf, n);
+                  os.write (buf, n);
+                }
+              }
+
+              os.close ();
+
+              if (!pr.wait ())
+                fail << "tar " << *pr.exit;
+            }
+            catch (const system_error& e)
+            {
+              // There must be something wrong with the setup or there is no
+              // space left on the disk, thus the failure is fatal.
+              //
+              fail << "unable to archive " << td << i << "/: " << e;
+            }
+
+            // Post the upload instance archive.
+            //
+            using namespace http_service;
+
+            parameters params ({
+                {parameter::text, "session",   tr.session},
+                {parameter::text, "instance",  i},
+                {parameter::file, "archive",   ari.path.string ()},
+                {parameter::text, "sha256sum", sha.string ()}});
+
+            if (challenge)
+              params.push_back ({
+                  parameter::text, "challenge", base64_encode (*challenge)});
+
+            result pr (post (ops, url, params));
+
+            // Turn the potential upload failure into the "upload" operation
+            // error, amending the task result manifest.
+            //
+            if (pr.error)
+            {
+              // The "upload" operation result must be present (otherwise
+              // there would be nothing to upload). We can assume it is last.
+              //
+              assert (!rm.results.empty ());
+
+              operation_result& r (rm.results.back ());
+
+              // The "upload" operation result must be the last, if present.
+              //
+              assert (r.operation == "upload");
+
+              auto log = [&r, indent = false] (const string& t,
+                                               const string& l) mutable
+              {
+                if (indent)
+                  r.log += "  ";
+                else
+                  indent = true;
+
+                r.log += t;
+                r.log += ": ";
+                r.log += l;
+                r.log += '\n';
+              };
+
+              log ("error",
+                   "unable to upload " + td.leaf ().string () + '/' + i +
+                   " build artifacts");
+
+              log ("error", *pr.error);
+
+              if (!pr.message.empty ())
+                log ("reason", pr.message);
+
+              if (pr.reference)
+                log ("reference", *pr.reference);
+
+              for (const manifest_name_value& nv: pr.body)
+              {
+                if (!nv.name.empty ())
+                  log (nv.name, nv.value);
+              }
+
+              r.status  |= result_status::error;
+              rm.status |= r.status;
+
+              break;
+            }
+          }
+
+          // Bail out on the instance archive upload failure.
+          //
+          if (!rm.status)
+            break;
+        }
+        catch (const system_error& e)
+        {
+          fail << "unable to iterate over " << td << ": " << e;
+        }
+      }
+    }
+
+    result_status rs (rm.status);
 
     // Upload the result.
     //
     result_request_manifest rq {tr.session,
                                 move (challenge),
                                 agent_checksum,
-                                move (r)};
+                                move (rm)};
     {
       const string& u (*tr.result_url);
 
diff --git a/bbot/agent/http-service.cxx b/bbot/agent/http-service.cxx
new file mode 100644
index 0000000..28b4d94
--- /dev/null
+++ b/bbot/agent/http-service.cxx
@@ -0,0 +1,465 @@
+// file      : bbot/agent/http-service.cxx -*- C++ -*-
+// license   : MIT; see accompanying LICENSE file
+
+#include <bbot/agent/http-service.hxx>
+
+#include <cstdlib> // strtoul()
+
+#include <bbot/diagnostics.hxx>
+
+using namespace std;
+using namespace butl;
+
+namespace bbot
+{
+  namespace http_service
+  {
+    result
+    post (const agent_options& o, const string& u, const parameters& params)
+    {
+      tracer trace ("http_service::post");
+
+      using parser     = manifest_parser;
+      using parsing    = manifest_parsing;
+      using name_value = manifest_name_value;
+
+      // The overall plan is to post the data using the curl program, read
+      // the HTTP response status and content type, read and parse the body
+      // according to the content type, and obtain the result message and
+      // optional reference in case of both the request success and failure.
+      //
+      // The successful request response (HTTP status code 200) is expected to
+      // contain the result manifest (text/manifest content type). The faulty
+      // response (HTTP status code other than 200) can either contain the
+      // result manifest or a plain text error description (text/plain content
+      // type) or some other content (for example text/html). We will return
+      // the manifest message value, if available or the first line of the
+      // plain text error description or, as a last resort, construct the
+      // message from the HTTP status code and reason phrase. We will also
+      // return the error description if anything goes wrong with the HTTP
+      // request or the response manifest status value is not 200.
+      //
+      string message;
+      optional<uint16_t> status;  // Request result manifest status value.
+      optional<string> reference;
+      vector<name_value> body;
+      optional<string> error;
+
+      // None of the 3XX redirect code semantics assume automatic re-posting.
+      // We will treat all such codes as failures, adding the location header
+      // value to the message for troubleshooting.
+      //
+      optional<string> location;
+
+      // Convert the submit arguments to curl's --form* options and cache the
+      // pointer to the file_text parameter value, if present, for writing
+      // into curl's stdin.
+      //
+      strings fos;
+      const string* file_text (nullptr);
+
+      for (const parameter& p: params)
+      {
+        if (p.type == parameter::file_text)
+        {
+          assert (file_text == nullptr);
+          file_text = &p.value;
+        }
+
+        fos.emplace_back (p.type == parameter::file ||
+                          p.type == parameter::file_text
+                          ? "--form"
+                          : "--form-string");
+
+        fos.emplace_back (
+          p.type == parameter::file      ? p.name + "=@" + p.value :
+          p.type == parameter::file_text ? p.name + "=@-"          :
+          p.name + '='  + p.value);
+      }
+
+      // Note that we prefer the low-level process API for running curl over
+      // using butl::curl because in this context it is restrictive and
+      // inconvenient.
+      //
+      // Start curl program.
+      //
+      // Text mode seems appropriate.
+      //
+      fdpipe in_pipe;
+      fdpipe out_pipe;
+      process pr;
+
+      try
+      {
+        in_pipe = fdopen_pipe ();
+
+        out_pipe = (file_text != nullptr
+                    ? fdopen_pipe ()
+                    : fdpipe {fdopen_null (), nullfd});
+
+        pr = process_start_callback (trace,
+                                     out_pipe.in.get () /* stdin  */,
+                                     in_pipe            /* stdout */,
+                                     2                  /* stderr */,
+                                     "curl",
+
+                                     // Include the response headers in the
+                                     // output so we can get the status
+                                     // code/reason, content type, and the
+                                     // redirect location.
+                                     //
+                                     "--include",
+
+                                     "--max-time", o.request_timeout (),
+                                     "--connect-timeout", o.connect_timeout (),
+                                     fos,
+                                     u);
+
+        // Shouldn't throw, unless something is severely damaged.
+        //
+        in_pipe.out.close ();
+        out_pipe.in.close ();
+      }
+      catch (const process_error& e)
+      {
+        fail << "unable to execute curl: " << e;
+      }
+      catch (const io_error& e)
+      {
+        fail << "unable to open pipe: " << e;
+      }
+
+      auto finish = [&pr, &error] (bool io_read = false, bool io_write = false)
+      {
+        if (!pr.wait ())
+          error = "curl " + to_string (*pr.exit);
+        else if (io_read)
+          error = "error reading curl output";
+        else if (io_write)
+          error = "error writing curl input";
+      };
+
+      bool io_write (false);
+      bool io_read  (false);
+
+      try
+      {
+        // First we read the HTTP response status line and headers. At this
+        // stage we will read until the empty line (containing just CRLF). Not
+        // being able to reach such a line is an error, which is the reason
+        // for the exception mask choice.
+        //
+        ifdstream is (
+          move (in_pipe.in),
+          fdstream_mode::skip,
+          ifdstream::badbit | ifdstream::failbit | ifdstream::eofbit);
+
+        if (file_text != nullptr)
+        {
+          ofdstream os (move (out_pipe.out));
+          os << *file_text;
+          os.close ();
+
+          // Indicate to the potential IO error handling that we are done with
+          // writing.
+          //
+          file_text = nullptr;
+        }
+
+        // Parse and return the HTTP status code. Return 0 if the argument is
+        // invalid.
+        //
+        auto status_code = [] (const string& s)
+        {
+          char* e (nullptr);
+          unsigned long c (strtoul (s.c_str (), &e, 10)); // Can't throw.
+          assert (e != nullptr);
+
+          return *e == '\0' && c >= 100 && c < 600
+                 ? static_cast<uint16_t> (c)
+                 : 0;
+        };
+
+        // Read the CRLF-terminated line from the stream stripping the
+        // trailing CRLF.
+        //
+        auto read_line = [&is] ()
+        {
+          string l;
+          getline (is, l); // Strips the trailing LF (0xA).
+
+          // Note that on POSIX CRLF is not automatically translated into
+          // LF, so we need to strip CR (0xD) manually.
+          //
+          if (!l.empty () && l.back () == '\r')
+            l.pop_back ();
+
+          return l;
+        };
+
+        auto bad_response = [] (const string& d) {throw runtime_error (d);};
+
+        // Read and parse the HTTP response status line, return the status
+        // code and the reason phrase.
+        //
+        struct http_status
+        {
+          uint16_t code;
+          string reason;
+        };
+
+        auto read_status = [&read_line, &status_code, &bad_response] ()
+        {
+          string l (read_line ());
+
+          for (;;) // Breakout loop.
+          {
+            if (l.compare (0, 5, "HTTP/") != 0)
+              break;
+
+            size_t p (l.find (' ', 5));             // The protocol end.
+            if (p == string::npos)
+              break;
+
+            p = l.find_first_not_of (' ', p + 1);   // The code start.
+            if (p == string::npos)
+              break;
+
+            size_t e (l.find (' ', p + 1));         // The code end.
+            if (e == string::npos)
+              break;
+
+            uint16_t c (status_code (string (l, p, e - p)));
+            if (c == 0)
+              break;
+
+            string r;
+            p = l.find_first_not_of (' ', e + 1);   // The reason start.
+            if (p != string::npos)
+            {
+              e = l.find_last_not_of (' ');         // The reason end.
+              assert (e != string::npos && e >= p);
+
+              r = string (l, p, e - p + 1);
+            }
+
+            return http_status {c, move (r)};
+          }
+
+          bad_response ("invalid HTTP response status line '" + l + '\'');
+
+          assert (false); // Can't be here.
+          return http_status {};
+        };
+
+        // The curl output for a successfull request looks like this:
+        //
+        // HTTP/1.1 100 Continue
+        //
+        // HTTP/1.1 200 OK
+        // Content-Length: 83
+        // Content-Type: text/manifest;charset=utf-8
+        //
+        // : 1
+        // status: 200
+        // message: submission is queued
+        // reference: 256910ca46d5
+        //
+        // curl normally sends the 'Expect: 100-continue' header for uploads,
+        // so we need to handle the interim HTTP server response with the
+        // continue (100) status code.
+        //
+        // Interestingly, Apache can respond with the continue (100) code and
+        // with the not found (404) code afterwords. Can it be configured to
+        // just respond with 404?
+        //
+        http_status rs (read_status ());
+
+        if (rs.code == 100)
+        {
+          while (!read_line ().empty ()) ; // Skips the interim response.
+          rs = read_status ();             // Reads the final status code.
+        }
+
+        // Read through the response headers until the empty line is
+        // encountered and obtain the content type and/or the redirect
+        // location, if present.
+        //
+        optional<string> ctype;
+
+        // Check if the line contains the specified header and return its
+        // value if that's the case. Return nullopt otherwise.
+        //
+        // Note that we don't expect the header values that we are interested
+        // in to span over multiple lines.
+        //
+        string l;
+        auto header = [&l] (const char* name) -> optional<string>
+        {
+          size_t n (string::traits_type::length (name));
+          if (!(icasecmp (name, l, n) == 0 && l[n] == ':'))
+            return nullopt;
+
+          string r;
+          size_t p (l.find_first_not_of (' ', n + 1)); // The value begin.
+          if (p != string::npos)
+          {
+            size_t e (l.find_last_not_of (' '));       // The value end.
+            assert (e != string::npos && e >= p);
+
+            r = string (l, p, e - p + 1);
+          }
+
+          return optional<string> (move (r));
+        };
+
+        while (!(l = read_line ()).empty ())
+        {
+          if (optional<string> v = header ("Content-Type"))
+            ctype = move (v);
+          else if (optional<string> v = header ("Location"))
+          {
+            if ((rs.code >= 301 && rs.code <= 303) || rs.code == 307)
+              location = move (v);
+          }
+        }
+
+        assert (!eof (is)); // Would have already failed otherwise.
+
+        // Now parse the response payload if the content type is specified and
+        // is recognized (text/manifest or text/plain), skip it (with the
+        // ifdstream's close() function) otherwise.
+        //
+        // Note that eof and getline() fail conditions are not errors anymore,
+        // so we adjust the exception mask accordingly.
+        //
+        is.exceptions (ifdstream::badbit);
+
+        if (ctype)
+        {
+          if (icasecmp ("text/manifest", *ctype, 13) == 0)
+          {
+            parser p (is, "manifest");
+            name_value nv (p.next ());
+
+            if (nv.empty ())
+              bad_response ("empty manifest");
+
+            const string& n (nv.name);
+            string& v (nv.value);
+
+            // The format version pair is verified by the parser.
+            //
+            assert (n.empty () && v == "1");
+
+            body.push_back (move (nv)); // Save the format version pair.
+
+            auto bad_value = [&p, &nv] (const string& d) {
+              throw parsing (p.name (), nv.value_line, nv.value_column, d);};
+
+            // Get and verify the HTTP status.
+            //
+            nv = p.next ();
+            if (n != "status")
+              bad_value ("no status specified");
+
+            uint16_t c (status_code (v));
+            if (c == 0)
+              bad_value ("invalid HTTP status '" + v + '\'');
+
+            if (c != rs.code)
+              bad_value ("status " + v + " doesn't match HTTP response "
+                         "code " + to_string (rs.code));
+
+            // Get the message.
+            //
+            nv = p.next ();
+            if (n != "message" || v.empty ())
+              bad_value ("no message specified");
+
+            message = move (v);
+
+            // Try to get an optional reference.
+            //
+            nv = p.next ();
+
+            if (n == "reference")
+            {
+              if (v.empty ())
+                bad_value ("empty reference specified");
+
+              reference = move (v);
+
+              nv = p.next ();
+            }
+
+            // Save the remaining name/value pairs.
+            //
+            for (; !nv.empty (); nv = p.next ())
+              body.push_back (move (nv));
+
+            status = c;
+          }
+          else if (icasecmp ("text/plain", *ctype, 10) == 0)
+            getline (is, message); // Can result in the empty message.
+        }
+
+        is.close (); // Detect errors.
+
+        // The only meaningful result we expect is the manifest (status code
+        // is not necessarily 200). We unable to interpret any other cases and
+        // so report them as a bad response.
+        //
+        if (!status)
+        {
+          if (rs.code == 200)
+            bad_response ("manifest expected");
+
+          if (message.empty ())
+          {
+            message = "HTTP status code " + to_string (rs.code);
+
+            if (!rs.reason.empty ())
+              message += " (" + lcase (rs.reason) + ')';
+          }
+
+          if (location)
+            message += ", new location: " + *location;
+
+          bad_response ("bad server response");
+        }
+      }
+      catch (const io_error&)
+      {
+        // Presumably the child process failed and issued diagnostics so let
+        // finish() try to deal with that first.
+        //
+        (file_text != nullptr ? io_write : io_read) = true;
+      }
+      // Handle all parsing errors, including the manifest_parsing exception
+      // that inherits from the runtime_error exception.
+      //
+      // Note that the io_error class inherits from the runtime_error class,
+      // so this catch-clause must go last.
+      //
+      catch (const runtime_error& e)
+      {
+        finish (); // Sets the error variable on process failure.
+
+        if (!error)
+          error = e.what ();
+      }
+
+      if (!error)
+        finish (io_read, io_write);
+
+      assert (error || (status && !message.empty ()));
+
+      if (!error && *status != 200)
+        error = "status code " + to_string (*status);
+
+      return result {
+        move (error), move (message), move (reference), move (body)};
+    }
+  }
+}
diff --git a/bbot/agent/http-service.hxx b/bbot/agent/http-service.hxx
new file mode 100644
index 0000000..b50c6b7
--- /dev/null
+++ b/bbot/agent/http-service.hxx
@@ -0,0 +1,71 @@
+// file      : bbot/agent/http-service.hxx -*- C++ -*-
+// license   : MIT; see accompanying LICENSE file
+
+#ifndef BBOT_AGENT_HTTP_SERVICE_HXX
+#define BBOT_AGENT_HTTP_SERVICE_HXX
+
+#include <libbutl/manifest-types.hxx>
+
+#include <bbot/types.hxx>
+#include <bbot/utility.hxx>
+
+#include <bbot/agent/agent-options.hxx>
+
+// NOTE: this implementation is inspired by the bdep's http_service::post()
+// function. The key difference is the result::error member which is used
+// to return rather than fail on upload errors.
+
+namespace bbot
+{
+  namespace http_service
+  {
+    // If type is file, then the value is a path to be uploaded. If type is
+    // file_text, then the value is a file content to be uploaded.
+    //
+    struct parameter
+    {
+      enum {text, file, file_text} type;
+      string name;
+      string value;
+    };
+    using parameters = vector<parameter>;
+
+    struct result
+    {
+      // If error is present, then it contains the description of why the
+      // upload failed. In this case message contains additional information.
+      //
+      optional<string> error;
+      string           message;
+      optional<string> reference;
+
+      // Does not include status, message, or reference.
+      //
+      vector<butl::manifest_name_value> body;
+    };
+
+    // Submit text parameters and/or upload files to an HTTP service via the
+    // POST method. Use the multipart/form-data content type if any files are
+    // uploaded and application/x-www-form-urlencoded otherwise.
+    //
+    // Note: currently only one file_text parameter can be specified.
+    //
+    // Return the response manifest message and reference (if present, see
+    // below) and the rest of the manifest values, if any. If unable to
+    // retrieve the response manifest, the message can also be set to the
+    // first line of the plain text error description or, as a last resort,
+    // constructed from the HTTP status code and reason phrase. Issue
+    // diagnostics and throw failed if something is wrong with the setup
+    // (unable to execute curl, etc).
+    //
+    // Note that the HTTP service is expected to respond with the result
+    // manifest that starts with the 'status' (HTTP status code) and 'message'
+    // (diagnostics message) values optionally followed by 'reference' and
+    // then other manifest values.
+    //
+    result
+    post (const agent_options&, const string& url, const parameters&);
+  }
+}
+
+#endif // BBOT_AGENT_HTTP_SERVICE_HXX
diff --git a/bbot/utility.hxx b/bbot/utility.hxx
index 35136bd..9bc517c 100644
--- a/bbot/utility.hxx
+++ b/bbot/utility.hxx
@@ -4,11 +4,12 @@
 #ifndef BBOT_UTILITY_HXX
 #define BBOT_UTILITY_HXX
 
-#include <memory>   // make_shared()
-#include <string>   // to_string(), stoull()
-#include <utility>  // move(), forward(), declval(), make_pair()
-#include <cassert>  // assert()
-#include <iterator> // make_move_iterator()
+#include <memory>    // make_shared()
+#include <string>    // to_string(), stoull()
+#include <utility>   // move(), forward(), declval(), make_pair()
+#include <cassert>   // assert()
+#include <iterator>  // make_move_iterator()
+#include <algorithm> // *
 
 #include <libbutl/ft/lang.hxx>
 
diff --git a/bbot/worker/worker.cxx b/bbot/worker/worker.cxx
index 4895c76..c2ee4ba 100644
--- a/bbot/worker/worker.cxx
+++ b/bbot/worker/worker.cxx
@@ -12,7 +12,6 @@
 #include <cstring>   // strchr(), strncmp()
 #include <sstream>
 #include <iostream>
-#include <algorithm> // find(), find_if(), remove_if()
 
 #include <libbutl/b.hxx>
 #include <libbutl/pager.hxx>
diff --git a/doc/cli.sh b/doc/cli.sh
index 22fe0c2..404e4d6 100755
--- a/doc/cli.sh
+++ b/doc/cli.sh
@@ -96,6 +96,7 @@ function compile_doc () # <file> <prefix> <suffix>
 --html-epilogue-file doc-epilogue.xhtml \
 --link-regex '%bpkg([-.].+)%../../bpkg/doc/bpkg$1%' \
 --link-regex '%bpkg(#.+)?%../../bpkg/doc/build2-package-manager-manual.xhtml$1%' \
+--link-regex '%brep(#.+)?%../../brep/doc/build2-repository-interface-manual.xhtml$1%' \
 --output-prefix "$2" \
 --output-suffix "$3" \
 "$1"
diff --git a/doc/manual.cli b/doc/manual.cli
index eb48a31..e61e6ef 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -45,8 +45,8 @@ Let's now examine the workflow in the other direction, that is, from a worker
 to a controller. Once a build machine is booted (by the agent), the worker
 inside connects to the TFTP server running on the build host and downloads the
 \i{build task manifest}. It then proceeds to perform the build task and
-uploads the \i{build result manifest} (which includes build logs) to the TFTP
-server.
+uploads the \i{build artifacts archive}, if any, followed by the \i{build
+result manifest} (which includes build logs) to the TFTP server.
 
 Once an agent receives a build task for a specific build machine, it goes
 through the following steps. First, it creates a directory on its TFTP server
@@ -79,14 +79,18 @@ agents. In this case we say that the \i{controller act as an agent}. The
 controller may also be configured to monitor build sources, such as SCM
 repositories, directly in which case it generates build tasks itself.
 
-In this architecture the build results are propagated up the chain: from a
-worker, to its agent, to its controller, and so on. A controller that is the
-final destination of a build result uses email to notify interested parties of
-the outcome. For example, \c{brep} would send a notification to the package
-owner if the build failed. Similarly, a \c{bbot} controller that monitors a
-\cb{git} repository would send an email to a committer if their commit caused a
-build failure. The email would include a link (normally HTTP/HTTPS) to the
-build logs hosted by the controller.
+In this architecture the build results and optional build artifacts are
+propagated up the chain: from a worker, to its agent, to its controller, and
+so on. A controller that is the final destination of a build result uses email
+to notify interested parties of the outcome. For example, \c{brep} would send
+a notification to the package owner if the build failed. Similarly, a \c{bbot}
+controller that monitors a \cb{git} repository would send an email to a
+committer if their commit caused a build failure. The email would include a
+link (normally HTTP/HTTPS) to the build logs hosted by the controller. The
+build artifacts, such as generated binary distribution packages, are normally
+made available for the interested parties to download. See \l{brep#upload
+Build Artifacts Upload} for details on the \c{brep} controller's
+implementation of the build artifacts upload handling.
 
 \h#arch-machine-config|Configurations|
 
@@ -851,6 +855,7 @@ subsequent sections.
 session: <id>
 [challenge]: <text>
 [result-url]: <url>
+[*-upload-url]: <url>
 [agent-checksum]: <checksum>
 \
 
@@ -889,6 +894,18 @@ private key and then \c{base64}-encoding the result.
 The URL to POST (upload) the result request to.
 
 
+\h2#arch-task-res-upload-url|\c{*-upload-url}|
+
+\
+[*-upload-url]: <url>
+\
+
+The URLs to upload the build artifacts to, if any, via the HTTP \c{POST}
+method using the \c{multipart/form-data} content type (see \l{brep#upload
+Build Artifacts Upload} for details on the upload protocol). The substring
+matched by \c{*} in \c{*-upload-url} denotes the upload type.
+
+
 \h2#arch-task-res-agent-checksum|\c{agent-checksum}|
 
 \
diff --git a/tests/integration/testscript b/tests/integration/testscript
index 75b61cb..a608bef 100644
--- a/tests/integration/testscript
+++ b/tests/integration/testscript
@@ -417,7 +417,7 @@ end
 tftp = 127.0.0.1:55123
 
 a = $0
-+ sed -e 's/-agent$/-worker/' <"$0" | set w
++sed -e 's/-agent$/-worker/' <"$0" | set w
 
 : agent
 :