From 845220e5b3a1add6e853f3264afbc305edfb28f0 Mon Sep 17 00:00:00 2001
From: Phil <phil@estuary.dev>
Date: Mon, 5 Feb 2024 14:54:18 -0500
Subject: [PATCH 1/3] agent-sql: log errors when upserting draft specs

---
 crates/agent-sql/src/drafts.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/agent-sql/src/drafts.rs b/crates/agent-sql/src/drafts.rs
index 40c760be5d..9113ebc34d 100644
--- a/crates/agent-sql/src/drafts.rs
+++ b/crates/agent-sql/src/drafts.rs
@@ -20,6 +20,7 @@ pub async fn create(
     Ok(row.id)
 }
 
+#[tracing::instrument(err, skip(spec, txn))]
 pub async fn upsert_spec<S>(
     draft_id: Id,
     catalog_name: &str,
@@ -53,7 +54,6 @@ where
     )
     .fetch_one(&mut *txn)
     .await?;
-
     Ok(())
 }
 

From 52ced951e7b9ce874e030a86a3d3c3c9645dcb91 Mon Sep 17 00:00:00 2001
From: Phil <phil@estuary.dev>
Date: Mon, 5 Feb 2024 14:54:43 -0500
Subject: [PATCH 2/3] agent: do unicode normalization on discovered collection
 names

---
 crates/agent/src/discovers/specs.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/crates/agent/src/discovers/specs.rs b/crates/agent/src/discovers/specs.rs
index aac3b01784..005aed0c10 100644
--- a/crates/agent/src/discovers/specs.rs
+++ b/crates/agent/src/discovers/specs.rs
@@ -289,10 +289,10 @@ pub fn merge_collections(
 }
 
 fn normalize_recommended_name(name: &str) -> String {
-    let parts: Vec<_> = models::Collection::regex()
+    use itertools::Itertools;
+    let mut parts = models::Collection::regex()
         .find_iter(name)
-        .map(|m| m.as_str())
-        .collect();
+        .map(|m| models::collate::normalize(m.as_str().chars()).collect::<String>());
 
     parts.join("_")
 }
@@ -681,8 +681,10 @@ mod tests {
         for (name, expect) in [
             ("Foo", "Foo"),
             ("foo/bar", "foo/bar"),
+            ("Faſt/Carſ", "Fast/Cars"), // First form is denormalized, assert that it gets NFKC normalized
+            ("/", ""),                  // just documenting a weird edge case
             ("/foo/bar//baz/", "foo/bar_baz"), // Invalid leading, middle, & trailing slash.
-            ("#੫൬    , bar-_!", "੫൬_bar-_"),   // Invalid leading, middle, & trailing chars.
+            ("#੫൬    , bar-_!", "੫൬_bar-_"), // Invalid leading, middle, & trailing chars.
             ("One! two/_three", "One_two/_three"),
         ] {
             assert_eq!(

From b066716521c2a8d3db48c40fa50893a707bc5cd8 Mon Sep 17 00:00:00 2001
From: Phil <phil@estuary.dev>
Date: Tue, 6 Feb 2024 17:23:25 -0500
Subject: [PATCH 3/3] agent: error when discovered response is missing
 recommended_name

When the discovered response doesn't include a `recommended_name` for one
of the bindings, insert a draft error instead of crashing the agent.
This is really a connector bug, but we want the agent to be tolerant
of those.
---
 crates/agent/src/discovers.rs | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/crates/agent/src/discovers.rs b/crates/agent/src/discovers.rs
index 527f422d84..88b28743a6 100644
--- a/crates/agent/src/discovers.rs
+++ b/crates/agent/src/discovers.rs
@@ -248,6 +248,25 @@ impl DiscoverHandler {
             specs::parse_response(endpoint_config, image_name, image_tag, discover_output)
                 .context("converting discovery response into specs")?;
 
+        if discovered_bindings
+            .iter()
+            .any(|b| b.recommended_name.is_empty())
+        {
+            tracing::error!(
+                ?discovered_bindings,
+                %capture_name,
+                %draft_id,
+                %image_name,
+                %image_tag,
+                "connector discovered response includes a binding with an empty recommended_name"
+            );
+            return Ok(Err(vec![draft::Error {
+                catalog_name: capture_name.to_string(),
+                scope: None,
+                detail: "connector protocol error: a binding was missing 'recommended_name'. Please contact support for assistance".to_string(),
+            }]));
+        }
+
         // Catalog we'll build up with the merged capture and collections.
         let mut catalog = models::Catalog::default();