fix: five things between an OCR reply being built and arriving readable

All found by running the Email OCR workflow end to end for the first time -
nothing downstream of attachment extraction had ever executed, so each fix
uncovered the next.

A run reported "completed" while its only real work failed. This morning's
"all iterations failed means the run failed" rule went into the main graph walk
and not into the nested-loop path, and send_ocr_email sits in a loop over one
email's attachments inside a loop over emails: one iteration, it failed, and the
execution finished green with a tolerated error. Exactly the case the rule
exists for, one level deeper than it was written.

The send itself failed with "Failed sending data to the peer" once the
recipient was correct. A header may read `Szontágh Ferenc <a@b.com>`; the SMTP
envelope may not - RCPT TO and MAIL FROM take an address and nothing else, and
the display-name form is a malformed command the server answers by dropping the
connection. envelopeAddress strips it for the envelope; the header keeps it.
Two different "To"s, RFC 5322 and RFC 5321, and they had been fed the same
string. The test asserts both from one address.

A text attachment declared no charset, so a UTF-8 file arrived as Latin-1:
"Kárpát-medencei" shown as "Kárpát-medencei". The body part had always
declared it and the attachment part never did. Only added when the caller has
not said otherwise.

An expression that could not be evaluated produced nothing and said so only in
the runner's log. That is how the reply went out with no attachment: the
expression reached through a node that had been skipped, threw, yielded an
empty string, and the node reported success. Failures are now recorded on the
node as expressionWarnings, naming the expression and the error. Deliberately
not a failure - many expressions are optional lookups, and failing them would
break workflows that rely on an empty result - but no longer invisible.

An HTTP error now names the URL it dialled. curl says what went wrong and never
to whom, and a workflow builds its URL from an expression, a credential or a
configured base - so "Could not resolve hostname" could not be told apart from
a base URL that had evaluated to empty. An empty URL now says so in as many
words.

lib/common/mime_words.cpp

@@ -227,6 +227,22 @@ std::string MimeWords::encodeAddress(const std::string& address) {
     return name + " " + addr;
 }
 
+std::string MimeWords::envelopeAddress(const std::string& address) {
+    const size_t open = address.rfind('<');
+    const size_t close = address.rfind('>');
+    if (open != std::string::npos && close != std::string::npos && close > open) {
+        return address.substr(open + 1, close - open - 1);
+    }
+    std::string trimmed = address;
+    while (!trimmed.empty() && std::isspace(static_cast<unsigned char>(trimmed.front()))) {
+        trimmed.erase(trimmed.begin());
+    }
+    while (!trimmed.empty() && std::isspace(static_cast<unsigned char>(trimmed.back()))) {
+        trimmed.pop_back();
+    }
+    return trimmed;
+}
+
 std::string MimeWords::encodeParameter(const std::string& name, const std::string& value) {
     if (isAscii(value)) {
         return name + "=\"" + value + "\"";

lib/common/mime_words.hpp

@@ -37,6 +37,13 @@ public:
     // `name` is the parameter name, e.g. "filename" or "name".
     static std::string encodeParameter(const std::string& name, const std::string& value);
 
+    // The bare address for an SMTP envelope: `Name <a@b.com>` becomes
+    // `a@b.com`. MAIL FROM and RCPT TO take an address and nothing else -
+    // handing them the display-name form produces a malformed command and the
+    // server drops the connection, reported only as "Failed sending data to
+    // the peer".
+    static std::string envelopeAddress(const std::string& address);
+
     // Whether a string is plain ASCII, and so needs none of the above.
     static bool isAscii(const std::string& value);
 };

src/runner/engine/script_engine.cpp

@@ -1039,10 +1039,34 @@ static CurlResponse performHttpRequest(
     CURLcode res = curl_easy_perform(curl);
 
     if (res != CURLE_OK) {
+        // Name the URL. curl's own text says only what went wrong - "Could not
+        // resolve hostname", "Connection refused" - never to whom, and a
+        // workflow may build its URL from an expression, a credential or a
+        // configured base. Without the address, "Could not resolve hostname"
+        // cannot be told apart from a base URL that evaluated to empty, and
+        // the search starts by guessing which of several hosts was meant.
         std::string error_msg = curl_easy_strerror(res);
+
+        // Asked back from curl rather than reusing what we passed in, so a
+        // redirect or a rewritten URL is reported as the one actually dialled.
+        const char* effective = nullptr;
+        curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective);
+        const std::string attempted = (effective && *effective) ? effective : url;
+
+        std::string detail = "HTTP request failed: " + error_msg;
+        if (!attempted.empty()) {
+            detail += " (" + attempted + ")";
+        } else {
+            // An empty URL is itself the answer, and the most common way to
+            // arrive at "could not resolve" - a base URL that resolved to
+            // nothing leaves the path alone in the request.
+            detail += " (the URL was empty - a base URL or expression that "
+                      "evaluated to nothing)";
+        }
+
         if (header_list) curl_slist_free_all(header_list);
         curl_easy_cleanup(curl);
-        throw std::runtime_error("HTTP request failed: " + error_msg);
+        throw std::runtime_error(detail);
     }
 
     // Get status code

src/runner/smtp/smtp_client.cpp

@@ -125,8 +125,20 @@ std::string SmtpClient::buildMime(const Message& message, const std::string& mes
     mime << wrapBase64(smartbotic::common::StringUtils::base64Encode(message.body));
 
     for (const auto& attachment : message.attachments) {
-        const std::string type = attachment.mime_type.empty() ? "application/octet-stream"
-                                                              : attachment.mime_type;
+        std::string type = attachment.mime_type.empty() ? "application/octet-stream"
+                                                        : attachment.mime_type;
+
+        // A text attachment must say what charset it is in. Without it a client
+        // falls back to its own default - Latin-1 in practice - and UTF-8 text
+        // arrives as mojibake: "Kárpát-medencei" shown as "Kárpát-medencei".
+        // The body part has always declared this; an attachment never did.
+        //
+        // Only added when the caller has not said otherwise, so a workflow that
+        // deliberately sends some other encoding keeps it.
+        if (type.rfind("text/", 0) == 0 &&
+            type.find("charset") == std::string::npos) {
+            type += "; charset=UTF-8";
+        }
         mime << "--" << boundary << "\r\n";
         mime << "Content-Type: " << type << "; "
              << smartbotic::common::MimeWords::encodeParameter("name", attachment.filename)
@@ -188,13 +200,19 @@ Result<SendResult> SmtpClient::send(const Message& message) {
         curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L);
     }
 
-    const std::string sender = senderAddress();
+    // Stripped for the same reason as the recipients: the envelope takes an
+    // address, not a display name, even if the credential was configured with
+    // one.
+    const std::string sender = smartbotic::common::MimeWords::envelopeAddress(senderAddress());
     curl_easy_setopt(curl, CURLOPT_MAIL_FROM, sender.c_str());
 
     struct curl_slist* recipients = nullptr;
-    for (const auto& address : message.to) recipients = curl_slist_append(recipients, address.c_str());
-    for (const auto& address : message.cc) recipients = curl_slist_append(recipients, address.c_str());
-    for (const auto& address : message.bcc) recipients = curl_slist_append(recipients, address.c_str());
+    for (const auto& address : message.to)
+        recipients = curl_slist_append(recipients, smartbotic::common::MimeWords::envelopeAddress(address).c_str());
+    for (const auto& address : message.cc)
+        recipients = curl_slist_append(recipients, smartbotic::common::MimeWords::envelopeAddress(address).c_str());
+    for (const auto& address : message.bcc)
+        recipients = curl_slist_append(recipients, smartbotic::common::MimeWords::envelopeAddress(address).c_str());
     curl_easy_setopt(curl, CURLOPT_MAIL_RCPT, recipients);
 
     curl_easy_setopt(curl, CURLOPT_READFUNCTION, readPayload);

src/runner/workflow_engine.cpp

@@ -162,6 +162,13 @@ namespace {
 // thread running the node, so the message is handed back the same way.
 thread_local std::string t_disabled_reference_error;
 
+// Expressions that failed while evaluating one node's configuration. Collected
+// here for the same reason as the above - the evaluator is several calls below
+// the node execution and cannot reach the record itself - and drained onto the
+// node's result so a failure is visible in the execution rather than only in
+// the runner's log.
+thread_local std::vector<std::string> t_expression_warnings;
+
 static bool configMatchesHash(const std::string& hashed, const nlohmann::json& config) {
     try {
         return nlohmann::json::parse(hashed) == config;
@@ -304,6 +311,11 @@ nlohmann::json ExecutionResult::toJson() const {
         nr["output"] = keep_full_outputs ? result.output : truncateLargeValues(result.output);
         nr["error"] = result.error;
         nr["retryCount"] = result.retry_count;
+        // Only when there is something to say, so every other record is
+        // unchanged and nothing has to learn a new field to read one.
+        if (!result.expression_warnings.empty()) {
+            nr["expressionWarnings"] = result.expression_warnings;
+        }
         if (result.status == NodeStatus::Completed && !result.error.empty()) {
             ++tolerated_error_count;
         } else if (result.status == NodeStatus::Failed && status == ExecutionStatus::Completed) {
@@ -956,6 +968,8 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 // Evaluate expressions in node config
                 WorkflowNode evaluated_node = *node;
                 t_disabled_reference_error.clear();
+            t_expression_warnings.clear();
+                t_expression_warnings.clear();
                 nlohmann::json defaulted_config = node->config;
                 auto node_def_for_defaults = registry_.getNode(node->type);
                 if (node_def_for_defaults) {
@@ -997,6 +1011,12 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     node_result = executeNode(evaluated_node, input, result.execution_id, workflow,
                                                node_def_for_defaults);
                 }
+                // Whatever happened above, anything the evaluator could not
+                // work out belongs on this node's record.
+                if (!t_expression_warnings.empty()) {
+                    node_result.expression_warnings = t_expression_warnings;
+                    t_expression_warnings.clear();
+                }
 
                 marker_outcome = applyNodeMarkers(node_result, node_id, node_id, result,
                                                   overlay, overlay_ignored, -1, callback);
@@ -3606,6 +3626,14 @@ bool WorkflowEngine::executeLoopBody(
                                            body_node_def_for_defaults);
             }
 
+            // Same as the top-level walk: anything the evaluator could not
+            // work out belongs on this node's record, whichever branch above
+            // produced it.
+            if (!t_expression_warnings.empty()) {
+                body_result.expression_warnings = t_expression_warnings;
+                t_expression_warnings.clear();
+            }
+
             // Named for the loop that runs it and which pass this is - the
             // same tag a node outside a loop never carries at all.
             body_result.loop_node_id = loop_node_id;
@@ -3656,7 +3684,26 @@ bool WorkflowEngine::executeLoopBody(
                 inner_done_data["data"] = body_result.output.value("data", nlohmann::json::object());
                 body_result.output["done"] = inner_done_data;
 
-                if (!inner_success && !inner_ctx.continue_on_error) {
+                // The same rule the outer walk applies, and it was missing
+                // here: tolerating failures item by item is the point of
+                // continue_on_error, but a nested loop in which NOTHING
+                // succeeded is the iteration failing, not a tolerated failure.
+                //
+                // Without this a run reported completed while its only real
+                // work failed - one attachment, one iteration, the send failed,
+                // and the execution still finished green with a tolerated
+                // error. Exactly what the outer rule was written to stop.
+                const bool inner_nothing_succeeded =
+                    inner_ctx.iterations_run > 0 &&
+                    inner_ctx.iterations_failed == inner_ctx.iterations_run;
+                if (inner_nothing_succeeded && inner_ctx.continue_on_error) {
+                    LOG_WARN("Nested loop {} tolerates failures, but all {} of its iterations "
+                             "failed - failing the enclosing iteration rather than reporting "
+                             "success", body_node_id, inner_ctx.iterations_run);
+                }
+
+                if (!inner_success &&
+                    (!inner_ctx.continue_on_error || inner_nothing_succeeded)) {
                     inner_loop_left_early = true;
                 }
             }
@@ -4223,6 +4270,12 @@ nlohmann::json WorkflowEngine::evaluateJavaScriptExpression(
             t_disabled_reference_error = result.error;
         }
         LOG_WARN("JavaScript expression evaluation failed: {} - Error: {}", expression, result.error);
+        // Kept short: an expression can be long, and this is a pointer to the
+        // problem, not a transcript of it.
+        std::string shown = expression.size() > 160 ? expression.substr(0, 160) + "..."
+                                                    : expression;
+        t_expression_warnings.push_back("Expression did not evaluate, so it produced nothing: " +
+                                        shown + " - " + result.error);
         return nullptr;
     }
 }

src/runner/workflow_engine.hpp

@@ -81,6 +81,19 @@ struct NodeExecutionResult {
     int retry_count = 0;
     bool from_cache = false;  // True if output was from cached previous execution
 
+    // Expressions in this node's configuration that could not be evaluated.
+    //
+    // A failing expression yields nothing and the node carries on, which is
+    // usually right - half a workflow's expressions are optional lookups. What
+    // was wrong is that the failure appeared only as a line in the runner log:
+    // an OCR reply went out with no attachment because its expression reached
+    // through a node that had been skipped, the node reported success, and the
+    // execution record showed an empty field with no hint of why.
+    //
+    // Recorded rather than raised, so pass/fail semantics do not change for
+    // workflows that already rely on an empty result.
+    std::vector<std::string> expression_warnings;
+
     // Which loop this record was produced inside, and which pass of it. Set
     // only for a node that actually ran (or, for a disabled node, was skipped)
     // as part of a loop body; a node outside any loop leaves both unset, and

tests/cpp/mime_words_test.cpp

@@ -90,6 +90,42 @@ int main() {
           MimeWords::decode(MimeWords::encodeAddress("Szontágh Ferenc <a@b.com>")),
           "Szontágh Ferenc <a@b.com>");
 
+    // The envelope takes an address and nothing else. A display name in a
+    // RCPT TO is a malformed command, and the server answers by dropping the
+    // connection - reported as "Failed sending data to the peer", which names
+    // neither the address nor the reason.
+    check("envelopeAddress: strips a display name",
+          MimeWords::envelopeAddress("Szontágh Ferenc <a@b.com>"), "a@b.com");
+    check("envelopeAddress: a bare address is unchanged",
+          MimeWords::envelopeAddress("a@b.com"), "a@b.com");
+    check("envelopeAddress: trims surrounding space",
+          MimeWords::envelopeAddress("  a@b.com  "), "a@b.com");
+    check("envelopeAddress: strips a quoted display name",
+          MimeWords::envelopeAddress("\"Doe, John\" <a@b.com>"), "a@b.com");
+
+    // The pair that matters, from one address: the header keeps the display
+    // name (RFC 5322 name-addr, which is what the recipient's client shows),
+    // the envelope drops it (RFC 5321 takes an address and nothing else). They
+    // are two different "To"s and feeding one to the other is what broke the
+    // send: a display name in RCPT TO is a malformed command.
+    {
+        const std::string address = "Szontágh Ferenc <ferenc.szontagh@smartbotics.ai>";
+
+        // Header: the name survives, encoded because it is not ASCII, and a
+        // reader decodes it back to exactly what was meant.
+        const std::string header = MimeWords::encodeAddress(address);
+        check("header keeps the display name (decoded round trip)",
+              MimeWords::decode(header), address);
+        std::cout << (header.find("ferenc.szontagh@smartbotics.ai") != std::string::npos
+                          ? "  ok   " : "  FAIL ")
+                  << "header keeps the address untouched\n";
+        if (header.find("ferenc.szontagh@smartbotics.ai") == std::string::npos) ++failures;
+
+        // Envelope: address only.
+        check("envelope drops the display name",
+              MimeWords::envelopeAddress(address), "ferenc.szontagh@smartbotics.ai");
+    }
+
     // A filename gets the RFC 2231 form plus an ASCII fallback.
     const std::string param = MimeWords::encodeParameter("filename", "Sárkányok.pdf");
     check("encodeParameter: ascii is plain",

tests/nodes/expression-failure-is-recorded.json

@@ -0,0 +1,55 @@
+{
+  "name": "expression-failure-is-recorded",
+  "description": "An expression that cannot be evaluated yields nothing and the node carries on - which is usually right, since many expressions are optional lookups. What was wrong is that the failure appeared only in the runner's log: an OCR reply went out with no attachment because its expression reached through a node that had been skipped, the node reported success, and the execution record showed an empty field with no hint of why. The failure is now on the node's record, without changing whether the node passed.",
+  "nodes": [
+    {
+      "id": "n1",
+      "name": "Trigger",
+      "type": "click-trigger",
+      "position": {
+        "x": 0,
+        "y": 0
+      },
+      "config": {}
+    },
+    {
+      "id": "n2",
+      "name": "Broken",
+      "type": "set-fields",
+      "position": {
+        "x": 0,
+        "y": 100
+      },
+      "config": {
+        "mode": "only-set",
+        "fields": [
+          {
+            "name": "reaches_through_nothing",
+            "value": "{{ $node['No Such Node'].result.text }}"
+          },
+          {
+            "name": "fine",
+            "value": "ok"
+          }
+        ]
+      }
+    }
+  ],
+  "connections": [
+    {
+      "sourceNodeId": "n1",
+      "sourceOutput": "main",
+      "targetNodeId": "n2",
+      "targetInput": "data"
+    }
+  ],
+  "expect": {
+    "n2": {
+      "status": "completed",
+      "output": {
+        "fine": "ok",
+        "reaches_through_nothing": ""
+      }
+    }
+  }
+}