Split the autosync-paused hint into guidance + reason lines

fiskus · fiskus · commit c43d33ba0e42 · 2026-07-13T14:39:43.000+02:00
The paused reason now shows on its own line under a fixed guidance line
('resolve, then push manually to resume'); the backend stops appending
that guidance to the reason message, so the data carries only the raw
reason and each UI surface owns the guidance.
diff --git a/quilt-sync/src-tauri/src/autopull/reporter.rs b/quilt-sync/src-tauri/src/autopull/reporter.rs
@@ -57,32 +57,17 @@ pub struct PausedEvent {
     pub message: Option<String>,
 }
 
-/// Trailing hint we append to every `Other(_)` paused message.
-///
-/// Two distinct gotchas this addresses:
-/// 1. **Pause persistence.** Once a namespace is in `inner.paused` the
-///    watcher's tick `continue`s past it, so it never re-evaluates with
-///    fresh settings — the user must click an explicit action
-///    (Commit & Push / Pull / Push / Set Remote) for `clear_paused` to
-///    fire. Without this hint, users see the banner sitting there even
-///    after they've fixed the underlying issue and assume the app is
-///    stuck.
-/// 2. **Stale message.** The text in this banner is the error from the
-///    publish attempt that paused the namespace. If the user has since
-///    changed publish settings (via the Settings UI form, which does
-///    update the in-memory cache), the *current* state may no longer
-///    match what the message describes.
-const OTHER_PAUSED_HINT: &str = "Autopush is paused on this package — the watcher will not retry until you click \
-     Commit & Push manually. The message above may be stale if you have already updated \
-     publish settings.";
-
 impl PausedEvent {
+    /// `message` carries only the raw refusal reason. The "resolve, then
+    /// push manually to resume" guidance is presentation and lives in the
+    /// UI (it once was appended here, which mixed data with guidance and
+    /// made the banner unreadable).
     pub fn from_reason(namespace: &Namespace, reason: &PausedReason) -> Self {
         let (reason_str, message) = match reason {
             PausedReason::PendingChanges => ("pendingChanges", None),
             PausedReason::PendingCommit => ("pendingCommit", None),
             PausedReason::Diverged => ("diverged", None),
-            PausedReason::Other(msg) => ("other", Some(format!("{msg}. {OTHER_PAUSED_HINT}"))),
+            PausedReason::Other(msg) => ("other", Some(msg.clone())),
         };
         Self {
             namespace: namespace.to_string(),
@@ -280,42 +265,27 @@ mod tests {
     }
 
     #[test]
-    fn paused_event_from_reason_other_carries_message_with_hint() {
+    fn paused_event_from_reason_other_carries_raw_reason_only() {
         let ns = quilt_uri::Namespace::from(("acme", "demo"));
         let ev = PausedEvent::from_reason(
             &ns,
             &PausedReason::Other("workflow rejected metadata".to_string()),
         );
         assert_eq!(ev.reason, "other");
 
-        // The raw error string is preserved at the front of the
-        // message so the user sees what went wrong, and the
-        // restart-cached-settings hint is appended so they aren't
-        // stuck wondering why hand-editing publish_settings.json had
-        // no effect.
-        let msg = ev.message.as_deref().expect("Other should carry a message");
-        assert!(
-            msg.starts_with("workflow rejected metadata"),
-            "raw error should lead the message, got: {msg}"
-        );
-        assert!(
-            msg.contains("Autopush is paused"),
-            "hint should make the paused-direction explicit, got: {msg}"
-        );
-        assert!(
-            msg.contains("Commit & Push"),
-            "hint should name the retry action, got: {msg}"
-        );
-        assert!(
-            msg.contains("may be stale"),
-            "hint should warn the error string may be stale, got: {msg}"
+        // `message` is exactly the raw refusal reason — no appended
+        // guidance. The "resolve, then push manually" line is presentation
+        // and is added by each UI surface, not baked into the data.
+        assert_eq!(
+            ev.message.as_deref(),
+            Some("workflow rejected metadata"),
+            "Other should carry the raw reason with no appended hint"
         );
 
-        // Serializes as camelCase, with the full message (raw error + hint).
+        // Serializes as camelCase with the raw reason as the message.
         let json = serde_json::to_string(&ev).unwrap();
         assert!(json.contains(r#""reason":"other""#), "got: {json}");
         assert!(json.contains("workflow rejected metadata"), "got: {json}");
-        assert!(json.contains("Autopush is paused"), "got: {json}");
         assert!(json.contains(r#""namespace":"acme/demo""#), "got: {json}");
     }
 }
diff --git a/quilt-sync/ui/assets/css/views/status.css b/quilt-sync/ui/assets/css/views/status.css
@@ -24,6 +24,12 @@
     margin: 0;
   }
 
+  .detail {
+    color: var(--q-ui-palette-text-secondary);
+    font-size: 0.875rem;
+    margin: var(--q-ui-size-1) 0 0;
+  }
+
   .action {
     display: flex;
     gap: var(--q-ui-size-2);
diff --git a/quilt-sync/ui/src/pages/installed_package/status_banner.rs b/quilt-sync/ui/src/pages/installed_package/status_banner.rs
@@ -146,17 +146,19 @@ pub(super) fn StatusBanner(
                 // `InstalledPackage` filters everything else out so we
                 // don't double-banner Diverged / Behind / Ahead, which
                 // are already covered by the status-driven `content`
-                // below. `Other(_)` always populates `message` with the
-                // raw error plus the `OTHER_PAUSED_HINT` suffix, so we
-                // render it verbatim. The `unwrap_or_else` is a
-                // belt-and-braces fallback for malformed events.
-                let description = ev
-                    .message
-                    .unwrap_or_else(|| "Autosync paused".to_string());
+                // below. `message` carries just the raw refusal reason;
+                // the guidance line ("resolve, then push manually to
+                // resume") is presentation, added here.
+                let reason = ev.message;
                 view! {
                     <div class="qui-status">
                         <div class="root">
-                            <h2 class="description">{description}</h2>
+                            <div class="text">
+                                <h2 class="description">
+                                    "Autosync paused. Resolve the issue, then push manually to resume."
+                                </h2>
+                                {reason.map(|r| view! { <p class="detail">{r}</p> })}
+                            </div>
                         </div>
                     </div>
                 }
diff --git a/quilt-sync/ui/src/pages/installed_packages_list.rs b/quilt-sync/ui/src/pages/installed_packages_list.rs
@@ -41,24 +41,32 @@ type PausedMapSignal = RwSignal<HashMap<String, String>>;
 /// whether it has a catalog host configured, and any autosync pause
 /// message. Returns `None` for a healthy row (no third line, not red).
 ///
-/// Pure so it can be unit-tested. Mirrors the detail-page status banner:
-/// a `paused` row shows the refusal reason; an `error` row shows a
-/// sign-in or no-remote hint depending on whether a remote host exists.
-fn error_hint(status: &str, has_host: bool, paused_message: Option<&str>) -> Option<String> {
+/// The guidance line shown above a paused row's reason: paused rows stay
+/// paused until the user acts, so it names the resume action rather than
+/// leaving them to wonder why autosync stopped.
+const PAUSED_GUIDANCE: &str = "Autosync paused. Resolve the issue, then push manually to resume.";
+
+/// The one or two hint lines shown under a row's URI, error-coloured; empty
+/// when the row is healthy (which is also the not-red condition).
+///
+/// Pure so it can be unit-tested. A `paused` row shows the guidance line
+/// followed by the raw refusal reason (when one is known); an `error` row
+/// shows a sign-in or no-remote hint depending on whether a remote host
+/// exists.
+fn hint_lines(status: &str, has_host: bool, paused_message: Option<&str>) -> Vec<String> {
     if status == "paused" || paused_message.is_some() {
-        return Some(match paused_message {
-            Some(msg) => format!("Autosync paused: {msg}"),
-            None => "Autosync paused".to_string(),
-        });
+        let mut lines = vec![PAUSED_GUIDANCE.to_string()];
+        lines.extend(paused_message.map(str::to_string));
+        return lines;
     }
     if status == "error" {
-        return Some(if has_host {
+        return vec![if has_host {
             "Unable to check remote status — sign in again".to_string()
         } else {
             "No remote configured".to_string()
-        });
+        }];
     }
-    None
+    Vec::new()
 }
 
 // ── Installed Packages List page ──
@@ -330,14 +338,14 @@ fn PackageItem(
     let namespace_display = data.namespace.clone();
     let remote_display = data.remote_display.clone();
 
-    // Third-line attention hint. Red state and the reason line are both
-    // driven by `hint`: it is `Some` exactly when the row needs attention
-    // (autosync-paused, or a remote error), `None` for a healthy row.
+    // Attention hint lines under the URI. Red state and the lines are both
+    // driven by `hint`: it is non-empty exactly when the row needs attention
+    // (autosync-paused, or a remote error), empty for a healthy row.
     let has_host = data.uri.as_ref().and_then(util::host_str).is_some();
     let ns_for_hint = data.namespace.clone();
     let hint = Signal::derive(move || {
         let paused_message = paused_map.with(|map| map.get(&ns_for_hint).cloned());
-        status.with(|s| error_hint(s, has_host, paused_message.as_deref()))
+        status.with(|s| hint_lines(s, has_host, paused_message.as_deref()))
     });
 
     // Build menu buttons
@@ -353,10 +361,10 @@ fn PackageItem(
     );
 
     view! {
-        <li class=move || if hint.get().is_some() {
-            "qui-installed-package-item error"
-        } else {
+        <li class=move || if hint.with(Vec::is_empty) {
             "qui-installed-package-item"
+        } else {
+            "qui-installed-package-item error"
         }>
             <a class="link" href=pkg_href>
                 <span class="item-primary">{namespace_display}</span>
@@ -366,9 +374,9 @@ fn PackageItem(
                         {uri}
                     </span>
                 })}
-                {move || hint.get().map(|h| view! {
-                    <span class="item-error-hint">{h}</span>
-                })}
+                {move || hint.get().into_iter().map(|line| view! {
+                    <span class="item-error-hint">{line}</span>
+                }).collect::<Vec<_>>()}
             </a>
             <Show when=move || refreshing.get()>
                 <div class="q-spinner-inline" />
@@ -695,59 +703,65 @@ fn CreatePackagePopup(
 
 #[cfg(test)]
 mod tests {
-    use super::error_hint;
+    use super::{PAUSED_GUIDANCE, hint_lines};
 
     #[test]
-    fn paused_with_reason_shows_autosync_paused_hint() {
+    fn paused_with_reason_shows_guidance_then_reason() {
         assert_eq!(
-            error_hint("paused", true, Some("workflow rejected metadata")),
-            Some("Autosync paused: workflow rejected metadata".to_string())
+            hint_lines("paused", true, Some("workflow rejected metadata")),
+            vec![
+                PAUSED_GUIDANCE.to_string(),
+                "workflow rejected metadata".to_string(),
+            ]
         );
         // A snapshot-seeded pause carries its reason even when the row's
         // own status string was refreshed to something else on mount.
         assert_eq!(
-            error_hint("up_to_date", false, Some("hash mismatch")),
-            Some("Autosync paused: hash mismatch".to_string())
+            hint_lines("up_to_date", false, Some("hash mismatch")),
+            vec![PAUSED_GUIDANCE.to_string(), "hash mismatch".to_string()]
         );
     }
 
     #[test]
-    fn paused_without_reason_falls_back_to_generic() {
+    fn paused_without_reason_shows_guidance_only() {
         assert_eq!(
-            error_hint("paused", true, None),
-            Some("Autosync paused".to_string())
+            hint_lines("paused", true, None),
+            vec![PAUSED_GUIDANCE.to_string()]
         );
     }
 
     #[test]
-    fn paused_reason_takes_precedence_over_error_status() {
+    fn paused_takes_precedence_over_error_status() {
         // A row that is both `error` and has a pause reason shows the pause
-        // reason — the more specific, actionable message wins.
+        // guidance + reason — the more specific, actionable message wins.
         assert_eq!(
-            error_hint("error", true, Some("workflow rejected metadata")),
-            Some("Autosync paused: workflow rejected metadata".to_string())
+            hint_lines("error", true, Some("workflow rejected metadata")),
+            vec![
+                PAUSED_GUIDANCE.to_string(),
+                "workflow rejected metadata".to_string(),
+            ]
         );
     }
 
     #[test]
     fn error_with_host_prompts_sign_in() {
         assert_eq!(
-            error_hint("error", true, None),
-            Some("Unable to check remote status — sign in again".to_string())
+            hint_lines("error", true, None),
+            vec!["Unable to check remote status — sign in again".to_string()]
         );
     }
 
     #[test]
     fn error_without_host_reports_no_remote() {
         assert_eq!(
-            error_hint("error", false, None),
-            Some("No remote configured".to_string())
+            hint_lines("error", false, None),
+            vec!["No remote configured".to_string()]
         );
     }
 
     #[test]
     fn healthy_row_has_no_hint() {
-        assert_eq!(error_hint("up_to_date", true, None), None);
-        assert_eq!(error_hint("ahead", false, None), None);
+        assert!(hint_lines("up_to_date", true, None).is_empty());
+        assert!(hint_lines("ahead", false, None).is_empty());
     }
 }
