fix(acp): goal-unavailable notice gating on resume (#503)

## Summary

Goal-notice fixes for nori-client MCP goal resume, split out from the
original combined cleanup PR (the browser-CDP fixes now live in #505
targeting main).

- Surface the goal-unavailable notice for **all** in-play goals on
resume, not just the first.
- Stop recording the goal-unavailable notice to the transcript (it is a
transient UI notice, not conversation history).
- Document the goal-notice gating and non-recording-on-resume behavior.
- Share a panic-safe `EnvGuard` across the goal resume tests.

Targets `feat/nori-client-mcp-cleanup`.

## Test plan

- [x] `cargo test -p nori-acp` (574 pass, with the feat-branch mock
agent built)
- [x] `cargo clippy -p nori-acp --tests` (clean)
- [x] `cargo fmt --check`

Author: CSRessel

nori-rs/acp/docs.md

@@ -127,13 +127,13 @@ The model-facing tool contract is intentionally narrower than the user-facing `/
 
 Before user prompts are submitted to the ACP runtime, `user_input.rs` prepends the current goal as a structured `<goal_context>` block when a goal exists. Hook context is still applied before goal context, and compact summaries remain the outermost framing instruction, so resumed/compacted turns retain their existing prompt-ordering invariant while still carrying goal state to the agent. The prompt goal context and hidden continuation prompt use the same compact elapsed-time and token-count formatting as the visible TUI goal summary.
 
-Agents that are not advertised the local `nori-client` server do not receive goal context through prompt transformation and do not receive hidden goal-continuation prompts. Backend `ThreadGoal*` operations from user-facing paths emit an unavailable notice instead of mutating goal state, because those agents cannot call `update_goal` to close the loop. If transcript replay restores an active goal into a non-MCP session, resume emits the same unavailable notice after the replayed goal snapshot rather than silently leaving automation inert. The local MCP server is the required automation path for active goals, while transcript replay, usage accounting, and ordinary prompts continue without it. The intended degradation path is a concise first-prompt `<context>` block with Nori CLI context, the open source repo URL, and a note that MCP-backed Nori affordances are unavailable, not repeated prompt-prefix workarounds on every turn.
+Agents that are not advertised the local `nori-client` server do not receive goal context through prompt transformation and do not receive hidden goal-continuation prompts. Backend `ThreadGoal*` operations from user-facing paths emit an unavailable notice instead of mutating goal state, because those agents cannot call `update_goal` to close the loop. That op-time notice is emitted directly to the client channel and deliberately **not** recorded to the transcript -- like resume notices it is a transient affordance derived from session state, so recording it would replay and accumulate a duplicate notice on every `/goal` op. If transcript replay restores any in-play goal (active, paused, blocked, or usage-limited) into a non-MCP session, resume emits the same unavailable notice after the replayed goal snapshot rather than the `/goal resume` affordance, which would mislead since `/goal` is disabled for non-MCP agents. The local MCP server is the required automation path for active goals, while transcript replay, usage accounting, and ordinary prompts continue without it. The intended degradation path is a concise first-prompt `<context>` block with Nori CLI context, the open source repo URL, and a note that MCP-backed Nori affordances are unavailable, not repeated prompt-prefix workarounds on every turn.
 
 After an active goal mutation or a visible user prompt completes with `StopReason::EndTurn`, `session_runtime_driver.rs` may submit a hidden goal-continuation prompt to the same ACP session. `thread_goal.rs` owns the continuation prompt text so it is derived from the current backend goal snapshot, not from TUI state or transcript text. The driver only starts a continuation when the goal is active, the reducer has returned to idle, and no queued user work remains. Chaining from one hidden `GoalContinuation` into another is gated on `goal_mcp_connected`, an `@/nori-rs/acp/src/backend/mod.rs` session flag that `NoriClientService::initialize` (the rmcp `ServerHandler::initialize` hook) flips only after the local MCP server receives an `initialize` request. The first successful initialize also re-emits `SessionCapabilitiesChanged` with `nori_client.initialized = true`. This is a safety invariant: until the agent has actually connected to the `nori-client` endpoint it has no way to mark the goal complete, so unbounded continuation-to-continuation chaining is not allowed. Agents without a connected goal MCP endpoint receive at most one hidden continuation per active goal mutation or visible user turn.
 
 Goal state is also part of the replay contract. `transcript.rs` passes Nori-owned goal update and clear events through replay, and `session.rs` seeds `ThreadGoalState` from those transcript-derived events before ACP session setup advertises local MCP tools. Server-side `session/load` can also emit ACP replay notifications while loading; those normalized client events are deferred until backend setup completes, then combined with the transcript replay events before rebuilding `ThreadGoalState`. This ordering matters because ACP agents replay their own session history, but they do not replay Nori-owned `ThreadGoalUpdated` events, so the transcript remains authoritative for goal state even when the agent emits load replay notifications.
 
-When a resumed goal is paused, blocked, or usage-limited, `thread_goal.rs` derives a one-time `SessionUpdateInfo` notice from the rehydrated snapshot so the TUI can show why goal automation will not continue until the user resumes, edits, clears, or resolves the blocker. That notice is emitted directly by `session.rs` after deferred replay events and is not written back into the transcript, so each future resume still derives its notice from goal state instead of accumulating duplicate history entries. ACP usage updates still normalize to `SessionUpdateInfo`, but `session_runtime_driver.rs` observes those events and asks the goal state to refresh `tokens_used`; when a goal exists, the backend emits a follow-up `ThreadGoalUpdated` snapshot so the TUI and transcript stay synchronized with usage accounting.
+When a resumed goal is paused, blocked, or usage-limited **and goal automation is available**, `thread_goal.rs` derives a one-time `SessionUpdateInfo` resume notice from the rehydrated snapshot so the TUI can show why goal automation will not continue until the user resumes, edits, clears, or resolves the blocker. `ThreadGoalState::resume_notice_for` centralizes this decision: with automation available it returns the status-specific resume notice (suggesting `/goal resume`); without automation it returns the unavailable notice for any in-play goal instead, since the resume affordance would be misleading. Either notice is emitted directly by `session.rs` after deferred replay events and is not written back into the transcript, so each future resume still derives its notice from goal state instead of accumulating duplicate history entries. ACP usage updates still normalize to `SessionUpdateInfo`, but `session_runtime_driver.rs` observes those events and asks the goal state to refresh `tokens_used`; when a goal exists, the backend emits a follow-up `ThreadGoalUpdated` snapshot so the TUI and transcript stay synchronized with usage accounting.
 
 **Browser Session** (`backend/browser_session.rs`):
 

nori-rs/acp/src/backend/session.rs

@@ -485,15 +485,7 @@ impl AcpBackend {
         let resume_goal_notice = {
             let goals = backend.thread_goal_state.lock().await;
             let now = thread_goal::now_seconds();
-            if !goal_automation_available
-                && goals.snapshot(now).is_some_and(|goal| {
-                    goal.status == codex_protocol::protocol::ThreadGoalStatus::Active
-                })
-            {
-                Some(thread_goal::unavailable_notice())
-            } else {
-                goals.resume_notice(now)
-            }
+            goals.resume_notice_for(now, goal_automation_available)
         };
 
         if !deferred_replay_client_events.is_empty() || resume_goal_notice.is_some() {

nori-rs/acp/src/backend/tests/mod.rs

@@ -1,6 +1,45 @@
 use super::*;
 use serial_test::serial;
 
+/// RAII guard that sets or removes a process env var and restores the previous
+/// value on drop, including on panic/unwind. `#[serial]` tests share process
+/// env, so a non-restored mutation would leak into later tests.
+struct EnvGuard {
+    name: &'static str,
+    previous: Option<String>,
+}
+
+impl EnvGuard {
+    fn set(name: &'static str, value: &str) -> Self {
+        let previous = std::env::var(name).ok();
+        unsafe {
+            std::env::set_var(name, value);
+        }
+        Self { name, previous }
+    }
+
+    fn remove(name: &'static str) -> Self {
+        let previous = std::env::var(name).ok();
+        unsafe {
+            std::env::remove_var(name);
+        }
+        Self { name, previous }
+    }
+}
+
+impl Drop for EnvGuard {
+    fn drop(&mut self) {
+        match &self.previous {
+            Some(value) => unsafe {
+                std::env::set_var(self.name, value);
+            },
+            None => unsafe {
+                std::env::remove_var(self.name);
+            },
+        }
+    }
+}
+
 async fn recv_backend_control(
     rx: &mut mpsc::Receiver<BackendEvent>,
     timeout: std::time::Duration,

nori-rs/acp/src/backend/tests/part4.rs

@@ -1618,8 +1618,10 @@ async fn test_resume_session_uses_server_side_when_load_session_succeeds() {
     }
 }
 
-/// When transcript replay restores a paused goal, the resumed session should
-/// surface a direct notice with the next available goal commands.
+/// When transcript replay restores a paused goal into an MCP-capable session,
+/// the resumed session should surface a direct notice with the next available
+/// goal commands. The `/goal resume` affordance is only valid when goal
+/// automation is available, so the agent must advertise HTTP MCP support.
 #[tokio::test]
 #[serial]
 async fn test_resume_session_notifies_about_paused_goal() {
@@ -1635,6 +1637,8 @@ async fn test_resume_session_notifies_about_paused_goal() {
         return;
     }
 
+    let _mcp_guard = EnvGuard::set("MOCK_AGENT_MCP_HTTP", "1");
+
     let temp_dir = tempfile::tempdir().expect("Failed to create temp dir");
     let (backend_event_tx, mut backend_event_rx) = mpsc::channel(64);
     let config = build_test_config(temp_dir.path());
@@ -1713,16 +1717,7 @@ async fn test_resume_session_non_mcp_active_goal_reports_unavailable() {
         return;
     }
 
-    let previous_mcp_http = std::env::var("MOCK_AGENT_MCP_HTTP").ok();
-    unsafe {
-        std::env::remove_var("MOCK_AGENT_MCP_HTTP");
-    }
-    let restore_mcp_http = || unsafe {
-        match &previous_mcp_http {
-            Some(value) => std::env::set_var("MOCK_AGENT_MCP_HTTP", value),
-            None => std::env::remove_var("MOCK_AGENT_MCP_HTTP"),
-        }
-    };
+    let _mcp_guard = EnvGuard::remove("MOCK_AGENT_MCP_HTTP");
 
     let temp_dir = tempfile::tempdir().expect("Failed to create temp dir");
     let (backend_event_tx, mut backend_event_rx) = mpsc::channel(64);
@@ -1775,14 +1770,12 @@ async fn test_resume_session_non_mcp_active_goal_reports_unavailable() {
                     saw_replayed_goal,
                     "unavailable notice should follow the replayed goal snapshot"
                 );
-                restore_mcp_http();
                 return;
             }
             Some(_) => continue,
             None => continue,
         }
     }
 
-    restore_mcp_http();
     panic!("Timed out waiting for active-goal unavailable notice");
 }

nori-rs/acp/src/backend/tests/part5.rs

@@ -1,10 +1,5 @@
 use super::*;
 
-struct EnvGuard {
-    name: &'static str,
-    previous: Option<String>,
-}
-
 struct RegistryGuard;
 
 impl RegistryGuard {
@@ -20,37 +15,6 @@ impl Drop for RegistryGuard {
     }
 }
 
-impl EnvGuard {
-    fn set(name: &'static str, value: &str) -> Self {
-        let previous = std::env::var(name).ok();
-        unsafe {
-            std::env::set_var(name, value);
-        }
-        Self { name, previous }
-    }
-
-    fn remove(name: &'static str) -> Self {
-        let previous = std::env::var(name).ok();
-        unsafe {
-            std::env::remove_var(name);
-        }
-        Self { name, previous }
-    }
-}
-
-impl Drop for EnvGuard {
-    fn drop(&mut self) {
-        match &self.previous {
-            Some(value) => unsafe {
-                std::env::set_var(self.name, value);
-            },
-            None => unsafe {
-                std::env::remove_var(self.name);
-            },
-        }
-    }
-}
-
 async fn assert_no_prompt_completed(
     backend_event_rx: &mut mpsc::Receiver<BackendEvent>,
     window: std::time::Duration,
@@ -621,6 +585,81 @@ async fn non_mcp_replayed_active_goal_does_not_drive_prompt_context_or_continuat
     );
 }
 
+#[tokio::test]
+#[serial]
+async fn non_mcp_goal_op_notice_is_not_recorded_to_transcript() {
+    use std::time::Duration;
+
+    let mock_config =
+        crate::registry::get_agent_config("mock-model").expect("mock-model should be registered");
+    if !std::path::Path::new(&mock_config.command).exists() {
+        eprintln!(
+            "Skipping test: mock_acp_agent not found at {}",
+            mock_config.command
+        );
+        return;
+    }
+
+    let _mcp_guard = EnvGuard::remove("MOCK_AGENT_MCP_HTTP");
+
+    let temp_dir = tempfile::tempdir().expect("Failed to create temp dir");
+    let (backend_event_tx, mut backend_event_rx) = mpsc::channel(64);
+    let config = build_test_config(temp_dir.path());
+
+    let backend = AcpBackend::spawn(&config, backend_event_tx)
+        .await
+        .expect("Failed to spawn ACP backend");
+
+    let _ = recv_backend_control(&mut backend_event_rx, Duration::from_secs(5))
+        .await
+        .expect("Should receive SessionConfigured event");
+
+    let recorder = backend
+        .transcript_recorder
+        .clone()
+        .expect("transcript recorder should be initialized");
+
+    // Repeated /goal ops in a non-MCP session each surface the unavailable
+    // notice to the user.
+    for _ in 0..2 {
+        backend
+            .submit(Op::ThreadGoalGet)
+            .await
+            .expect("Failed to submit /goal get");
+
+        let start = std::time::Instant::now();
+        let mut saw_unavailable = false;
+        while start.elapsed() < Duration::from_secs(5) {
+            match recv_backend_client(&mut backend_event_rx, Duration::from_millis(500)).await {
+                Some(nori_protocol::ClientEvent::SessionUpdateInfo(update))
+                    if update.message.contains("/goal is unavailable") =>
+                {
+                    saw_unavailable = true;
+                    break;
+                }
+                Some(_) => {}
+                None => {}
+            }
+        }
+        assert!(
+            saw_unavailable,
+            "expected unavailable notice emitted to the client"
+        );
+    }
+
+    // The notice is a transient affordance (like resume notices), so it must
+    // never be persisted to the transcript, where it would replay and
+    // accumulate on every resume.
+    recorder.flush().await.expect("flush transcript");
+    let recorded = tokio::fs::read_to_string(recorder.transcript_path())
+        .await
+        .expect("read transcript");
+    assert!(
+        !recorded.contains("/goal is unavailable"),
+        "goal-unavailable notice must not be recorded to the transcript, got:\n{recorded}"
+    );
+}
+
 #[tokio::test]
 #[serial]
 async fn setting_active_goal_starts_hidden_continuation_without_extra_prompt() {

nori-rs/acp/src/backend/thread_goal.rs

@@ -179,6 +179,27 @@ Before deciding that the goal is achieved, treat completion as unproven and veri
         }
     }
 
+    /// Resume-time notice that respects whether goal automation is available.
+    /// Without HTTP MCP support `/goal` is disabled, so the `resume_notice`
+    /// affordances (which suggest `/goal resume`) would mislead; surface the
+    /// `unavailable_notice` for any goal that is still in play instead.
+    pub(crate) fn resume_notice_for(
+        &self,
+        now: i64,
+        goal_automation_available: bool,
+    ) -> Option<SessionUpdateInfo> {
+        if goal_automation_available {
+            return self.resume_notice(now);
+        }
+        self.snapshot(now).and_then(|goal| match goal.status {
+            ThreadGoalStatus::Active
+            | ThreadGoalStatus::Paused
+            | ThreadGoalStatus::Blocked
+            | ThreadGoalStatus::UsageLimited => Some(unavailable_notice()),
+            ThreadGoalStatus::BudgetLimited | ThreadGoalStatus::Complete => None,
+        })
+    }
+
     pub(crate) fn set_objective(
         &mut self,
         objective: String,
@@ -454,12 +475,15 @@ impl AcpBackend {
     }
 
     async fn emit_goal_unavailable(&self) {
-        emit_client_event(
-            &self.backend_event_tx,
-            self.transcript_recorder.as_ref(),
-            ClientEvent::SessionUpdateInfo(unavailable_notice()),
-        )
-        .await;
+        // Deliberately not recorded to the transcript: like resume notices this
+        // is a transient affordance derived from session state. Recording it
+        // would replay and accumulate a duplicate notice on every /goal op.
+        let _ = self
+            .backend_event_tx
+            .send(BackendEvent::Client(ClientEvent::SessionUpdateInfo(
+                unavailable_notice(),
+            )))
+            .await;
     }
 
     async fn emit_thread_goal_updated(&self, goal: ThreadGoalSnapshot) {
@@ -695,6 +719,55 @@ mod tests {
         assert_eq!(goals.resume_notice(55), None);
     }
 
+    #[test]
+    fn resume_notice_for_non_mcp_surfaces_unavailable_for_in_play_goals() {
+        let mut goals = ThreadGoalState::default();
+        // No goal: nothing to surface, regardless of automation availability.
+        assert_eq!(goals.resume_notice_for(10, true), None);
+        assert_eq!(goals.resume_notice_for(10, false), None);
+
+        goals
+            .set_objective("Keep going".to_string(), Some(ThreadGoalStatus::Active), 10)
+            .expect("valid objective");
+        // Active goal with automation available has no resume affordance...
+        assert_eq!(goals.resume_notice_for(15, true), None);
+        // ...but without automation we surface that /goal is unavailable.
+        assert_eq!(
+            goals.resume_notice_for(15, false),
+            Some(unavailable_notice())
+        );
+
+        // Every resumable status surfaces the unavailable notice without
+        // automation, and never the misleading /goal resume affordance.
+        for status in [
+            ThreadGoalStatus::Paused,
+            ThreadGoalStatus::Blocked,
+            ThreadGoalStatus::UsageLimited,
+        ] {
+            goals.set_status(status, 20).expect("existing goal");
+            let available = goals
+                .resume_notice_for(25, true)
+                .expect("resumable goal notice");
+            assert!(
+                available
+                    .hint
+                    .as_deref()
+                    .is_some_and(|hint| hint.contains("/goal resume"))
+            );
+            assert_eq!(
+                goals.resume_notice_for(25, false),
+                Some(unavailable_notice())
+            );
+        }
+
+        // Terminal statuses surface nothing in either mode.
+        goals
+            .set_status(ThreadGoalStatus::Complete, 30)
+            .expect("existing goal");
+        assert_eq!(goals.resume_notice_for(35, true), None);
+        assert_eq!(goals.resume_notice_for(35, false), None);
+    }
+
     #[test]
     fn usage_updates_count_tokens_since_goal_started() {
         let mut goals = ThreadGoalState::default();