a review-classifier key in the wrong shape or rejected by anthropic logs how to fix it

Cargo.lock

@@ -5337,6 +5337,7 @@ dependencies = [
  "tempfile",
  "tokio",
  "tracing",
+ "vaultrs",
 ]
 
 [[package]]

swarm-review-classifier/Cargo.toml

@@ -36,6 +36,8 @@ tracing.workspace = true
 tempfile = "3"
 # `InMemoryMetricExporter` for the cost counter test.
 opentelemetry_sdk = { workspace = true, features = ["testing"] }
+# Builds the decode failure the store client reports for a wrong-shape secret.
+vaultrs.workspace = true
 
 [lints]
 workspace = true

swarm-review-classifier/src/run.rs

@@ -164,7 +164,7 @@ impl KeySource for Store {
         match runtime {
             Runtime::ApiKey => {
                 let path = api_key_path().map_err(|e| e.to_string())?;
-                let key = read_key(&path, "API key", |k: ApiKey| k.api_key).await?;
+                let key = read_key(&path, "API key", "api_key", |k: ApiKey| k.api_key).await?;
                 key.ok_or_else(|| {
                     format!(
                         "no API key at secret/{path}: write it in the secret store's UI, field \
@@ -174,7 +174,10 @@ impl KeySource for Store {
             }
             Runtime::Acp(_) => {
                 let path = acp_provider_key_path().map_err(|e| e.to_string())?;
-                let key = read_key(&path, "ACP provider key", |k: ProviderKey| k.api_key).await?;
+                let key = read_key(&path, "ACP provider key", "api_key", |k: ProviderKey| {
+                    k.api_key
+                })
+                .await?;
                 key.ok_or_else(|| {
                     format!(
                         "no ACP provider key at secret/{path}: write it in the secret store's \
@@ -215,11 +218,27 @@ fn read_failure(path: &str, e: &dyn std::error::Error) -> String {
     format!("reading secret/{path}: {e}")
 }
 
+/// The run failure for a store read that failed with `e`, where the object at
+/// `path` should hold the string field `field`. An object that does not decode
+/// gets a message naming the shape and the write that fixes it, without `e`:
+/// its sources quote the stored value.
+fn read_error(path: &str, field: &str, e: &swarm_secret_client::Error) -> String {
+    if e.is_undecodable() {
+        format!(
+            "secret/{path} does not hold a string field `{field}`: write it with `bao kv put \
+             secret/{path} {field}=-`"
+        )
+    } else {
+        read_failure(path, e)
+    }
+}
+
 /// The key `field` takes from the object at `path`, trimmed; `None` when
 /// nothing or a blank is stored.
 async fn read_key<T: serde::de::DeserializeOwned>(
     path: &str,
     what: &str,
+    field_name: &str,
     field: impl FnOnce(T) -> String,
 ) -> Result<Option<String>, String> {
     let store = swarm_secret_client::SecretStore::from_env(crate::CERT_ROLE)
@@ -228,7 +247,7 @@ async fn read_key<T: serde::de::DeserializeOwned>(
     let stored: Option<T> = store
         .read_optional(path)
         .await
-        .map_err(|e| read_failure(path, &e))?;
+        .map_err(|e| read_error(path, field_name, &e))?;
     Ok(stored
         .map(field)
         .map(|key| key.trim().to_owned())
@@ -260,7 +279,19 @@ impl Model for Claude {
         if let Err(e) = tokio::fs::remove_dir_all(&dir).await {
             tracing::warn!(dir = %dir.display(), error = %e, "removing a run's directory failed");
         }
-        outcome
+        outcome.map_err(key_rejection)
+    }
+}
+
+/// `error` as the run failure for a model call: when the Anthropic API
+/// refused the key (HTTP 401), it says where to rotate it, followed by the
+/// original text. That text names the status and no key.
+fn key_rejection(error: String) -> String {
+    match api_key_path() {
+        Ok(path) if error.contains("(HTTP 401)") => format!(
+            "the Anthropic API key was rejected (HTTP 401): rotate it at secret/{path} ({error})"
+        ),
+        _ => error,
     }
 }
 
@@ -449,6 +480,48 @@ mod tests {
         assert_eq!(text, "reading secret/p: error parsing the response");
     }
 
+    #[test]
+    fn an_undecodable_secret_names_the_shape_and_leaves_out_the_value() {
+        let cause = serde_json::from_str::<u32>("\"hunter2\"").unwrap_err();
+        assert!(
+            cause.to_string().contains("hunter2"),
+            "control: the source quotes the value"
+        );
+        let undecodable =
+            swarm_secret_client::Error::from(vaultrs::error::ClientError::JsonParseError {
+                source: cause,
+            });
+        let text = read_error("swarm/p", "api_key", &undecodable);
+        assert_eq!(
+            text,
+            "secret/swarm/p does not hold a string field `api_key`: write it with `bao kv put \
+             secret/swarm/p api_key=-`"
+        );
+        // The control: any other store failure keeps the generic text.
+        let other = swarm_secret_client::Error::MissingEnv("BAO_ADDR");
+        assert_eq!(
+            read_error("swarm/p", "api_key", &other),
+            "reading secret/swarm/p: BAO_ADDR is unset or empty"
+        );
+    }
+
+    #[test]
+    fn a_rejected_key_says_where_to_rotate_it() {
+        let path = api_key_path().unwrap();
+        assert_eq!(
+            key_rejection("claude failed: authentication failed (HTTP 401)".to_owned()),
+            format!(
+                "the Anthropic API key was rejected (HTTP 401): rotate it at secret/{path} \
+                 (claude failed: authentication failed (HTTP 401))"
+            )
+        );
+        // The control: other failures pass through.
+        assert_eq!(
+            key_rejection("claude failed: exit 1".to_owned()),
+            "claude failed: exit 1"
+        );
+    }
+
     #[test]
     fn chain_renders_every_level() {
         let e = Level(

swarm-secret-client/src/lib.rs

@@ -140,6 +140,18 @@ impl Error {
             Self::Vault(e) if matches!(**e, vaultrs::error::ClientError::APIError { code: 403, .. })
         )
     }
+
+    /// Whether the store answered, but what it holds did not decode as the
+    /// type the caller asked for: a field is missing or has another type.
+    /// The error's own text quotes the stored value, so a caller that reports
+    /// this case writes its own message rather than showing the error.
+    #[must_use]
+    pub fn is_undecodable(&self) -> bool {
+        matches!(
+            self,
+            Self::Vault(e) if matches!(**e, vaultrs::error::ClientError::JsonParseError { .. })
+        )
+    }
 }
 
 /// Install `aws-lc-rs` as the process-wide rustls crypto provider. Call it
@@ -175,6 +187,22 @@ mod tests {
         assert!(!Error::MissingEnv("BAO_ADDR").is_denied());
     }
 
+    #[test]
+    fn only_a_decode_failure_is_undecodable() {
+        let decode =
+            serde_json::from_value::<String>(serde_json::json!({ "value": "k" })).unwrap_err();
+        let undecodable =
+            Error::from(vaultrs::error::ClientError::JsonParseError { source: decode });
+        assert!(undecodable.is_undecodable());
+        // The controls: a denial and a local failure are not decode failures.
+        let denied = Error::from(vaultrs::error::ClientError::APIError {
+            code: 403,
+            errors: Vec::new(),
+        });
+        assert!(!denied.is_undecodable());
+        assert!(!Error::MissingEnv("BAO_ADDR").is_undecodable());
+    }
+
     #[test]
     fn a_tls_client_config_builds_once_the_provider_is_installed() {
         install_crypto_provider();