fix: secret ordering, dead-end opens, and stale response metrics

crates/engine/src/auth/mod.rs

@@ -72,3 +72,26 @@ pub fn apply_to_http(
     http::bind(request, &cred);
     Ok(true)
 }
+
+/// Whether [`apply_to_http`] would bind a credential — answered *without* reading the credential's
+/// own `{{secret:…}}` fields, so the send path can decide whether the destination is even allowed
+/// one before paying for a secret-backend read. OAuth2 still consults the store (a cached token is
+/// what makes it a credential at all); that lookup resolves the *config*, never fails a send, and
+/// the send path has already primed it.
+pub fn would_bind(
+    auth: &Auth,
+    env: &Environment,
+    secrets: &dyn SecretResolver,
+    store: Option<&dyn TokenStore>,
+    auth_base_url: &str,
+) -> bool {
+    if !auth.is_active() {
+        return false;
+    }
+    if auth.kind == AuthKind::OAuth2 {
+        return store
+            .and_then(|s| oauth::cached_token(auth, env, secrets, s, auth_base_url))
+            .is_some();
+    }
+    true
+}

crates/engine/src/policy.rs

@@ -173,6 +173,25 @@ pub fn engaged_allow_list(
     Some(hosts)
 }
 
+/// Whether guarded material may leave for `url`: `true` when the allow-list isn't engaged, when
+/// the host matches it, or when the URL has no parseable host (nothing [`evaluate`] could report
+/// either). Decided from the destination alone, so the send path can answer it *before* resolving
+/// a credential — a host the policy refuses must never cost a secret-backend read.
+pub fn egress_allowed(
+    url: &str,
+    ctx: &EffectiveContext,
+    dynamics: &Dynamics,
+    extra_allowed: &[String],
+) -> bool {
+    let Some(allowed) = engaged_allow_list(ctx, dynamics, extra_allowed) else {
+        return true;
+    };
+    match host_of(url) {
+        Some(host) => allowed.iter().any(|p| host_matches(&host, p)),
+        None => true,
+    }
+}
+
 /// Evaluate the policy for a resolved send. `extra_allowed` are caller-granted one-off hosts
 /// (the CLI's `--allow-host`); pass none for a strict env — strictness is the caller's contract.
 ///
@@ -194,16 +213,12 @@ pub fn evaluate(
         });
     }
 
-    if material.guarded() {
+    if material.guarded() && !egress_allowed(url, ctx, dynamics, extra_allowed) {
         if let Some(host) = host_of(url) {
-            if let Some(allowed) = engaged_allow_list(ctx, dynamics, extra_allowed) {
-                if !allowed.iter().any(|p| host_matches(&host, p)) {
-                    out.push(PolicyViolation::OffHostEgress {
-                        host,
-                        material: material.clone(),
-                    });
-                }
-            }
+            out.push(PolicyViolation::OffHostEgress {
+                host,
+                material: material.clone(),
+            });
         }
     }
 

crates/engine/src/resolve.rs

@@ -174,11 +174,18 @@ impl ResolvedRequest {
     }
 
     /// The send-path entry once an [`EffectiveContext`] exists: resolve like
-    /// [`Self::resolve_effective_with`], but bind the credential unconditionally and *report*
-    /// policy violations (off-list guarded egress, plain-HTTP under `require_https`) instead of
-    /// silently withholding. The caller decides what a violation means — confirm dialog (UI),
-    /// hard error (CLI) — and must not send the returned request while ignoring the violations.
-    /// `extra_allowed` are caller-granted one-off hosts; pass none for a strict env.
+    /// [`Self::resolve_effective_with`], but decide the credential against the environment's
+    /// allow-list and *report* policy violations (off-list guarded egress, plain-HTTP under
+    /// `require_https`) instead of silently withholding. The caller decides what a violation means
+    /// — confirm dialog (UI), hard error (CLI) — and must not send the returned request while
+    /// ignoring the violations. `extra_allowed` are caller-granted one-off hosts; pass none for a
+    /// strict env.
+    ///
+    /// The allow-list decision comes *first*: a credential the policy won't admit is never
+    /// resolved, so an off-list destination costs no secret-backend read and can't fail the send
+    /// over a credential it would never have carried. The reported violation still names the
+    /// credential — it describes what a granted send would carry, which is what the caller is
+    /// being asked about.
     pub fn resolve_effective_checked(
         op: &Operation,
         ctx: &EffectiveContext,
@@ -193,15 +200,21 @@ impl ResolvedRequest {
         let recording = crate::policy::RecordingSecrets::new(secrets);
         let mut resolved = Self::resolve_operation_with(op, &ctx.env, &recording, dynamics)?;
         merge_scope_headers(&mut resolved, ctx, &recording, dynamics)?;
-        let cred_secrets = crate::policy::RecordingSecrets::new(secrets);
-        let credential = auth::apply_to_http(
-            &mut resolved,
-            &ctx.auth,
-            &ctx.env,
-            &cred_secrets,
-            store,
-            &ctx.auth_base_url,
-        )?;
+
+        let credential =
+            if crate::policy::egress_allowed(&resolved.url, ctx, dynamics, extra_allowed) {
+                let cred_secrets = crate::policy::RecordingSecrets::new(secrets);
+                auth::apply_to_http(
+                    &mut resolved,
+                    &ctx.auth,
+                    &ctx.env,
+                    &cred_secrets,
+                    store,
+                    &ctx.auth_base_url,
+                )?
+            } else {
+                auth::would_bind(&ctx.auth, &ctx.env, secrets, store, &ctx.auth_base_url)
+            };
         resolved.apply_defaults(&ctx.defaults);
 
         let material = crate::policy::EgressMaterial {
@@ -415,8 +428,8 @@ mod tests {
         assert_eq!(header(&r, "authorization"), Some("Bearer abc"));
         assert!(v.is_empty());
 
-        // Off-host: the credential is still bound (the caller decides), and the violation names
-        // both the destination and the guarded material.
+        // Off-host: the credential is withheld (the caller decides whether to grant the host), and
+        // the violation names both the destination and the material a granted send would carry.
         let op: Operation = Request::get("https://evil.test/x").into();
         let (r, v) = ResolvedRequest::resolve_effective_checked(
             &op,
@@ -427,7 +440,7 @@ mod tests {
             &[],
         )
         .unwrap();
-        assert_eq!(header(&r, "authorization"), Some("Bearer abc"));
+        assert_eq!(header(&r, "authorization"), None);
         match &v[..] {
             [PolicyViolation::OffHostEgress { host, material }] => {
                 assert_eq!(host, "evil.test");
@@ -440,8 +453,8 @@ mod tests {
             other => panic!("expected one off-host violation, got {other:?}"),
         }
 
-        // A one-off allowance admits the host.
-        let (_, v) = ResolvedRequest::resolve_effective_checked(
+        // A one-off allowance admits the host — and only then is the credential resolved.
+        let (r, v) = ResolvedRequest::resolve_effective_checked(
             &op,
             &collection_ctx(),
             &secrets,
@@ -451,6 +464,93 @@ mod tests {
         )
         .unwrap();
         assert!(v.is_empty());
+        assert_eq!(header(&r, "authorization"), Some("Bearer abc"));
+    }
+
+    /// A [`SecretResolver`] that fails like an unreachable backend and counts every lookup — the
+    /// ordering guarantee is "zero calls", which only a counting double can prove.
+    struct UnavailableSecrets(std::cell::Cell<usize>);
+
+    impl UnavailableSecrets {
+        fn new() -> Self {
+            Self(std::cell::Cell::new(0))
+        }
+        fn calls(&self) -> usize {
+            self.0.get()
+        }
+    }
+
+    impl SecretResolver for UnavailableSecrets {
+        fn resolve_secret(&self, _name: &str) -> Result<String, String> {
+            self.0.set(self.0.get() + 1);
+            Err("OS keychain secret storage is unavailable".into())
+        }
+    }
+
+    #[test]
+    fn off_list_host_neither_reads_nor_attaches_the_credential() {
+        let secrets = UnavailableSecrets::new();
+        let d = Dynamics::new();
+        // 127.0.0.1 is outside the env's derived hosts, so the inherited bearer never applies —
+        // an unreachable secret backend must not fail a send that wouldn't carry the secret.
+        let op: Operation = Request::get("http://127.0.0.1:8765/x").into();
+        let (r, v) = ResolvedRequest::resolve_effective_checked(
+            &op,
+            &collection_ctx(),
+            &secrets,
+            None,
+            &d,
+            &[],
+        )
+        .unwrap();
+        assert_eq!(secrets.calls(), 0, "the secret backend must not be touched");
+        assert_eq!(header(&r, "authorization"), None);
+        assert_eq!(header(&r, "x-org"), Some("acme"));
+        assert!(matches!(
+            &v[..],
+            [crate::policy::PolicyViolation::OffHostEgress { material, .. }] if material.strippable()
+        ));
+    }
+
+    #[test]
+    fn dropping_the_credential_clears_the_violation_off_list() {
+        let secrets = UnavailableSecrets::new();
+        // What the UI's "Send without credentials" resolves: nothing guarded is left, so the send
+        // proceeds to the off-list host with neither a credential nor a violation.
+        let mut ctx = collection_ctx();
+        ctx.auth = Auth::default();
+        let op: Operation = Request::get("http://127.0.0.1:8765/x").into();
+        let (r, v) = ResolvedRequest::resolve_effective_checked(
+            &op,
+            &ctx,
+            &secrets,
+            None,
+            &Dynamics::new(),
+            &[],
+        )
+        .unwrap();
+        assert_eq!(secrets.calls(), 0);
+        assert!(v.is_empty());
+        assert_eq!(header(&r, "authorization"), None);
+    }
+
+    #[test]
+    fn granting_the_host_resolves_the_credential_and_surfaces_backend_failure() {
+        let secrets = UnavailableSecrets::new();
+        // Once the caller admits the host the credential is genuinely needed, so an unavailable
+        // backend errors — today's error UX, now only where the secret is actually required.
+        let op: Operation = Request::get("http://127.0.0.1:8765/x").into();
+        let err = ResolvedRequest::resolve_effective_checked(
+            &op,
+            &collection_ctx(),
+            &secrets,
+            None,
+            &Dynamics::new(),
+            &["127.0.0.1".into()],
+        )
+        .unwrap_err();
+        assert_eq!(secrets.calls(), 1);
+        assert!(matches!(err, ResolveError::Auth(_)));
     }
 
     #[test]

crates/storage/src/lib.rs

@@ -74,6 +74,15 @@ pub enum StorageError {
     UnsafeSidecar { path: PathBuf, name: String },
 }
 
+impl StorageError {
+    /// Whether this failure is "the file/directory isn't there" — as opposed to unreadable,
+    /// malformed, or a bad tree operation. Callers use it to tell a *gone* path (worth forgetting)
+    /// from one that's merely broken right now (worth keeping and retrying).
+    pub fn is_not_found(&self) -> bool {
+        matches!(self, StorageError::Io { source, .. } if source.kind() == std::io::ErrorKind::NotFound)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -852,4 +861,18 @@ mod tests {
         let folder_id = reloaded.id_at_path("users").unwrap();
         assert_eq!(names_at(&reloaded, Some(folder_id)), ["Beta", "Alpha"]);
     }
+
+    /// Opening a workspace whose folder is gone must be distinguishable from opening a broken one —
+    /// that's what lets the recents list retire dead entries and keep the rest.
+    #[test]
+    fn loading_a_vanished_workspace_reports_not_found() {
+        let dir = tempfile::tempdir().unwrap();
+        let root = dir.path().join("gone");
+        let err = Workspace::load(&root).unwrap_err();
+        assert!(err.is_not_found(), "{err}");
+
+        let dir = tempfile::tempdir().unwrap();
+        std::fs::write(dir.path().join("_posel.toml"), "name = [").unwrap();
+        assert!(!Workspace::load(dir.path()).unwrap_err().is_not_found());
+    }
 }

crates/storage/src/ui_state.rs

@@ -89,6 +89,14 @@ impl UiState {
         self.recent_workspaces.truncate(RECENT_LIMIT);
     }
 
+    /// Drop `path` from the recents, reporting whether it was there. Used to retire an entry whose
+    /// folder is gone, so a dead row can't linger in the list forever.
+    pub fn forget_workspace(&mut self, path: &Path) -> bool {
+        let before = self.recent_workspaces.len();
+        self.recent_workspaces.retain(|p| p != path);
+        self.recent_workspaces.len() != before
+    }
+
     /// Remember a request's open sub-tabs, keyed by its absolute path.
     pub fn set_request_view(&mut self, key: impl Into<String>, view: RequestViewState) {
         self.request_views.insert(key.into(), view);
@@ -174,6 +182,16 @@ mod tests {
         assert_eq!(ui.recent_workspaces[0], PathBuf::from("/ws/19"));
     }
 
+    #[test]
+    fn forget_workspace_removes_only_the_named_entry() {
+        let mut ui = UiState::default();
+        ui.record_workspace(Path::new("/a"));
+        ui.record_workspace(Path::new("/b"));
+        assert!(ui.forget_workspace(Path::new("/a")));
+        assert_eq!(ui.recent_workspaces, vec![PathBuf::from("/b")]);
+        assert!(!ui.forget_workspace(Path::new("/a")));
+    }
+
     #[test]
     fn score_decays_by_half_each_half_life() {
         let mut ui = UiState::default();

crates/ui/src/auth_editor.rs

@@ -464,8 +464,6 @@ impl RequestView {
     /// check doesn't engage), the host is allowed, or the URL can't be resolved/parsed yet
     /// (partial URLs while typing shouldn't flash a warning).
     fn inherited_host_mismatch(&self, cx: &App) -> Option<String> {
-        use posel_engine::policy::{derived_hosts, host_matches};
-
         let ctx = self.effective_ctx(cx);
         if !ctx.auth.is_active() {
             return None;
@@ -476,18 +474,8 @@ impl RequestView {
             posel_engine::resolve_str_with(&raw_url, &ctx.env, &posel_engine::NoSecrets, dynamics)
                 .ok()?;
         let host = posel_engine::auth::host_of(&url)?;
-
-        let derived = derived_hosts(&ctx, dynamics);
-        if derived.is_empty() && ctx.policy.allowed_hosts.is_empty() {
-            return None;
-        }
-        let allowed = derived.iter().any(|p| host_matches(&host, p))
-            || ctx
-                .policy
-                .allowed_hosts
-                .iter()
-                .any(|p| host_matches(&host, p));
-        (!allowed).then_some(host)
+        // The same predicate the send path decides with, so the warning and the send never disagree.
+        (!posel_engine::policy::egress_allowed(&url, &ctx, dynamics, &[])).then_some(host)
     }
 
     /// The scope and scheme the active request's `Inherit` resolves to (for the read-out): the