Address review: every answered agent target fences a late Go

Only targets answered with a session were fenced against a delayed
generate event. A request that timed out (or ended in another failure
verdict the CLI already reported) was simply forgotten, so a Go whose
capture outlasted the timeout still opened a session nobody was told
about.

`resolve_agent_target` now records every terminal resolution, with the
answering session when the verdict carried one, and
`agent_target_refusal` refuses a generate event for any answered
target unless it comes from the answering session itself. The
browser_timeout instructions no longer send the agent to live-status
for a session that can no longer start. The overlay's refusal toast
covers both causes.

Tests: a Rust integration case and a Node protocol case (claim, time
out, late Go refused with 409 and nothing journaled), a unit test for
the timeout instruction; contract doc updated.

Written with AI assistance (Claude).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

crates/cli/tests/agent_target.rs

@@ -145,7 +145,10 @@ impl Server {
             .spawn()
             .expect("spawn live-server");
         let pid_file = dir.join(".impeccable/live/server.json");
-        assert!(wait_for(&pid_file, 10), "server pid file never appeared");
+        // Sixteen servers spawn at once under the default test parallelism; a
+        // loaded machine has taken more than ten seconds to write the first
+        // pid file.
+        assert!(wait_for(&pid_file, 30), "server pid file never appeared");
         let info: serde_json::Value =
             serde_json::from_str(&std::fs::read_to_string(&pid_file).unwrap()).unwrap();
         let port = info["port"].as_u64().expect("port") as u16;
@@ -559,3 +562,23 @@ fn agent_target_welcomes_the_generate_event_of_the_session_that_answered() {
     assert_eq!(status, 200, "{body}");
     assert!(s.dir.join(".impeccable/live/sessions/cccccccc.jsonl").exists());
 }
+
+#[test]
+fn agent_target_fences_a_generate_event_that_lands_after_the_timeout() {
+    let s = Server::start("fenced");
+    let mut a = Overlay::connect(s.port, &s.token, "tab-a");
+    a.next(|m| m["type"] == "connected");
+    let held = s.hold(serde_json::json!({}));
+    let target_id = a.next(|m| m["type"] == "agent_target")["targetId"].as_str().unwrap().to_string();
+    assert_eq!(s.claim(&target_id, "tab-a", true)["granted"], serde_json::json!(true));
+    // The holder never answers: the request times out (400ms) and the CLI
+    // reports it. Its Go lands after that: refused, nothing journaled, so
+    // no session exists that the agent was never told about.
+    let (_, verdict) = held.join().unwrap();
+    assert_eq!(verdict["error"], serde_json::json!("browser_timeout"), "{verdict}");
+    let (status, body) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "eeeeeeee", "tab-a"));
+    assert_eq!(status, 409, "{body}");
+    assert_eq!(body["error"], serde_json::json!("agent_target_already_served"));
+    assert!(body.get("sessionId").is_none(), "{body}");
+    assert!(!s.dir.join(".impeccable/live/sessions/eeeeeeee.jsonl").exists());
+}

crates/live/src/live_generate.rs

@@ -126,7 +126,7 @@ fn instructions_for(result: &Map<String, Value>, self_cmd: &str) -> Option<Strin
     }
     let text = match s("error").as_str() {
         "no_browser_connected" => "No page with the live overlay is connected. Open the app URL that serves a pageFiles entry yourself with your harness browser tool, then rerun this command. Only when no browser tool exists: give the user the URL and rerun with --wait-for-browser 120000 so the command fires as soon as they open the page.".to_string(),
-        "browser_timeout" => format!("The overlay did not answer in time. The page may be mid-reload: run {} live-status to check whether a session started anyway, reload the app page, then rerun this command.", self_cmd),
+        "browser_timeout" => "The overlay did not answer in time, and no session was started for this request (a Go that lands late is refused). The page may be mid-reload: reload the app page, then rerun this command.".to_string(),
         "invalid_selector" => "The selector is not valid CSS. Fix the selector syntax and rerun.".to_string(),
         "no_match" => {
             if n("rawMatchCount") > 0 {
@@ -366,6 +366,15 @@ mod tests {
         m
     }
 
+    #[test]
+    fn timeout_instructions_promise_no_stray_session() {
+        let mut m = Map::new();
+        m.insert("ok".into(), json!(false));
+        m.insert("error".into(), json!("browser_timeout"));
+        let text = instructions_for(&m, "impeccable").unwrap();
+        assert!(text.contains("no session was started"), "{text}");
+    }
+
     #[test]
     fn busy_instructions_tell_the_agent_whose_session_is_in_the_way() {
         let own = instructions_for(&busy("agent_target_in_flight"), "impeccable").unwrap();

crates/live/src/live_server.rs

@@ -184,7 +184,7 @@ pub fn run(args: &[String], io: &mut Io) -> i32 {
             next_apply_timer_gen: 0,
             pending_agent_targets: Vec::new(),
             next_agent_target_timer_gen: 0,
-            served_agent_targets: Vec::new(),
+            resolved_agent_targets: Vec::new(),
             shutting_down: false,
             cleaned_up: false,
             log_tx,
@@ -1240,17 +1240,19 @@ fn handle_events_post(
         }
     }
     if let Some(envelope) = agent_target.as_ref().and_then(Value::as_object) {
-        if let Some(served) = st.agent_target_served_elsewhere(envelope, id_str.as_deref()) {
+        if let Some(refusal) = st.agent_target_refusal(envelope, id_str.as_deref()) {
             // A superseded Go: this page's lease lapsed while it was still
-            // capturing and another page served the request. Journal
-            // nothing, so one request never gets two sessions.
+            // capturing and another page served the request, or the
+            // request was already answered (a timeout or a failure the CLI
+            // has reported). Journal nothing, so one request never gets a
+            // second session, or a session nobody was told about.
             drop(st);
             let mut body = json!({
                 "error": "agent_target_already_served",
                 "targetId": envelope.get("targetId").cloned().unwrap_or(Value::Null),
             });
-            if !served.is_empty() {
-                body["sessionId"] = Value::String(served);
+            if let Some(sid) = refusal.session_id {
+                body["sessionId"] = Value::String(sid);
             }
             respond(stream, cors, json_res(409, body));
             return;

crates/live/src/server_state.rs

@@ -45,6 +45,13 @@ pub struct SseClient {
 }
 
 /// One overlay's roll-call report on an agent target: its busy state and why.
+/// A generate event refused because its agent target is spoken for; see
+/// `ServerState::agent_target_refusal`.
+pub struct AgentTargetRefusal {
+    /// The session that answered the request, when the verdict carried one.
+    pub session_id: Option<String>,
+}
+
 pub struct AgentTargetReport {
     pub client_id: String,
     pub state: Value,
@@ -116,10 +123,12 @@ pub struct ServerState {
     /// Held-open agent targets keyed by targetId, in arrival order.
     pub pending_agent_targets: Vec<(String, AgentTargetPending)>,
     pub next_agent_target_timer_gen: u64,
-    /// Agent targets answered with a session, oldest first (bounded): a
-    /// generate event that names one of these under another session id is
-    /// a superseded Go and is refused.
-    pub served_agent_targets: Vec<(String, String)>,
+    /// Every agent target already answered, oldest first (bounded), with
+    /// the session that answered it when the verdict carried one: a
+    /// generate event that names one of these under another session id, or
+    /// after a verdict without a session (a timeout, a failure), is a
+    /// superseded Go and is refused.
+    pub resolved_agent_targets: Vec<(String, Option<String>)>,
     pub last_poll_at: i64,
     pub timed_out_apply_ids: Vec<(String, TimedOutApply)>,
     pub next_poll_id: u64,
@@ -820,30 +829,36 @@ impl ServerState {
             return false;
         };
         let (_, pending) = self.pending_agent_targets.remove(pos);
-        if result.get("ok") == Some(&Value::Bool(true)) {
-            if let Some(sid) = result.get("sessionId").and_then(Value::as_str) {
-                self.served_agent_targets
-                    .push((target_id.to_string(), sid.to_string()));
-                if self.served_agent_targets.len() > 64 {
-                    self.served_agent_targets.remove(0);
-                }
-            }
+        let session = if result.get("ok") == Some(&Value::Bool(true)) {
+            result
+                .get("sessionId")
+                .and_then(Value::as_str)
+                .map(str::to_string)
+        } else {
+            None
+        };
+        self.resolved_agent_targets
+            .push((target_id.to_string(), session));
+        if self.resolved_agent_targets.len() > 64 {
+            self.resolved_agent_targets.remove(0);
         }
         let _ = pending.tx.send(result);
         true
     }
 
-    /// Whether a generate event naming `envelope.targetId`, sent by
-    /// `envelope.clientId` under `session_id`, is a superseded Go: the
-    /// target is still pending but another page holds a live lease on it
-    /// (this page's lease lapsed while it was capturing), or the request
-    /// was already answered with a different session. Returns the serving
-    /// session id, empty while the rival has not minted one yet.
-    pub fn agent_target_served_elsewhere(
+    /// Why a generate event naming `envelope.targetId`, sent by
+    /// `envelope.clientId` under `session_id`, must not open a session:
+    /// the target is still pending but another page holds a live lease on
+    /// it (this page's lease lapsed while it was capturing), or the request
+    /// was already answered, with a different session or with none (a
+    /// timeout or a failure verdict the CLI has already reported). None
+    /// when the event is welcome, which includes the answering session's
+    /// own event.
+    pub fn agent_target_refusal(
         &self,
         envelope: &Map<String, Value>,
         session_id: Option<&str>,
-    ) -> Option<String> {
+    ) -> Option<AgentTargetRefusal> {
         let target_id = envelope.get("targetId").and_then(Value::as_str)?;
         let client_id = envelope
             .get("clientId")
@@ -856,17 +871,22 @@ impl ServerState {
         {
             return match &pending.owner {
                 Some(owner) if owner != client_id && pending.claimed_until > now_i64() => {
-                    Some(String::new())
+                    Some(AgentTargetRefusal { session_id: None })
                 }
                 _ => None,
             };
         }
-        self.served_agent_targets
+        let (_, answered_by) = self
+            .resolved_agent_targets
             .iter()
             .rev()
-            .find(|(t, _)| t == target_id)
-            .filter(|(_, sid)| Some(sid.as_str()) != session_id)
-            .map(|(_, sid)| sid.clone())
+            .find(|(t, _)| t == target_id)?;
+        if answered_by.as_deref() == session_id && session_id.is_some() {
+            return None;
+        }
+        Some(AgentTargetRefusal {
+            session_id: answered_by.clone(),
+        })
     }
 
     /// Every connected overlay has declined: answer busy now, not at the

docs/CLI-CONTRACT.md

@@ -1480,7 +1480,7 @@ Binds `127.0.0.1:PORT`. CORS: if request has `Origin` and (origin is loopback ht
 | `POST /manual-edit-discard?token=&pageUrl=` | 401 | see 10 |
 | `POST /manual-edit` | | 410 `{"error":"/manual-edit is removed; use /manual-edit-stash and /manual-edit-commit for staged copy edits."}` |
 | `POST /agent-target` | body JSON `token` mismatch → 401 `{"error":"Unauthorized"}`; invalid JSON → 400 `{"error":"Invalid JSON"}` | Agent-initiated targeting (the `generate` command). Validation (400 `{"error":<msg>}`, messages verbatim): `agent_target: selector is required`, `agent_target: selector too long` (>1000 chars), `agent_target: invalid action (valid: <VISUAL_ACTIONS joined ', '>)`, `agent_target: count must be 1-8`, `agent_target: text must be a string of at most 500 chars`, `agent_target: index must be a positive integer (1-based)`, `agent_target: prompt must be a string of at most 2000 chars`, `agent_target: dryRun must be a boolean`. No SSE client → 200 `{ok:false, error:'no_browser_connected'}`. Otherwise mint an 8-hex `targetId`, broadcast `agent_target` (see 6.2), and **hold the response** until `/agent-target-result` resolves it, every connected overlay has declined (busy roll call, see `/agent-target-claim`), or `IMPECCABLE_AGENT_TARGET_TIMEOUT_MS` (default 15000) elapses: busy verdict `{ok:false, error:'busy', state, reason}` from the first report when any report exists, else `{ok:false, error:'browser_timeout', timeoutMs}`. The held reply is 200 `{targetId, ...result}`; shutdown resolves every held request with `{ok:false, error:'server_stopping'}`. |
-| `POST /agent-target-result` | 401 / 400 Invalid JSON | `targetId` (non-empty string) required else 400 `{"error":"agent_target_result: missing targetId"}`; the remaining body fields (minus `token`) resolve the held request; 200 `{ok:true, delivered:boolean}` (`delivered:false` when nothing awaits that id). A `generate` event on `POST /events` may carry `agentTarget: {targetId, result}`: once the event is accepted, the server resolves that pending target with `result` (the envelope is stripped before journaling and never reaches the poller), so a page that dies between Go and its result cannot leave the request pending for a second Go elsewhere; whichever of the event and the result post lands first answers. The envelope also carries `clientId`: a generate event naming a target that another page now holds (a live lease, this page's having lapsed while it captured) or that was already answered with a different session is refused with 409 `{"error":"agent_target_already_served", targetId, sessionId?}` and journals nothing, and the overlay drops that local session. |
+| `POST /agent-target-result` | 401 / 400 Invalid JSON | `targetId` (non-empty string) required else 400 `{"error":"agent_target_result: missing targetId"}`; the remaining body fields (minus `token`) resolve the held request; 200 `{ok:true, delivered:boolean}` (`delivered:false` when nothing awaits that id). A `generate` event on `POST /events` may carry `agentTarget: {targetId, result}`: once the event is accepted, the server resolves that pending target with `result` (the envelope is stripped before journaling and never reaches the poller), so a page that dies between Go and its result cannot leave the request pending for a second Go elsewhere; whichever of the event and the result post lands first answers. The envelope also carries `clientId`: a generate event naming a target that another page now holds (a live lease, this page's having lapsed while it captured) or that was already answered, with a different session or with none (a timeout or a failure verdict the CLI has reported), is refused with 409 `{"error":"agent_target_already_served", targetId, sessionId?}` and journals nothing, and the overlay drops that local session; the answering session's own event is welcome. |
 | `POST /agent-target-claim` | 401 / 400 Invalid JSON | `targetId` and `clientId` (non-empty strings) required else 400 `{"error":"agent_target_claim: missing targetId or clientId"}`. Roll call plus a first-wins lease, so exactly one overlay acts on a broadcast target. Unknown or resolved target → `{ok:true, granted:false, pending:false}` (ends a rescuer's retry loop). `eligible !== true` → record `{state, reason, result?}` under `clientId` (replacing an earlier report; `result` is the overlay's resolution verdict when `reason` is `no_match`, i.e. its page cannot resolve the selector), release the lease if this client holds it, answer `{ok:true, granted:false, pending}` (`pending` false once the request resolved, so a declining overlay knows whether to keep watching for a change of its word), then complete the roll call when no owner holds the lease and reports ≥ connected overlays. Verdict precedence: a report whose `reason` is not `no_match` (a tab that could serve later) → `{ok:false, error:'busy', state, reason}` at once; when every report is `no_match` the roll call stays open for `IMPECCABLE_AGENT_TARGET_RESOLVE_GRACE_MS` (default 3000) after each overlay's first such report (a late reporter extends the grace by the full window; a page whose element mounts late keeps re-checking while its decline answers `pending:true`, an eligible claim drops its stale report, and the overlay declines rather than posting a result when the element is gone after its claim), then answers the first report's `result` (e.g. `no_match` with `rawMatchCount`, `invalid_selector`); the timeout uses the same precedence when any report exists. `eligible === true` → drop this client's earlier report; `granted` when no owner, the same owner (renew), or the lease lapsed (`IMPECCABLE_AGENT_TARGET_CLAIM_LEASE_MS`, default 3000); answer `{ok:true, granted, pending:true}`. |
 | anything else | | 404 `Not found` |
 

skill/scripts/live-browser.js

@@ -7736,9 +7736,10 @@
     }).then(async res => {
       if (res.ok) return res;
       const body = await res.json().catch(() => ({}));
-      // The helper refused to open a second session for an agent target
-      // another page already served (this page's lease lapsed while it was
-      // capturing): drop the local session and hand the surface back.
+      // The helper refused to open a session for an agent target it has
+      // already answered (another page served it after this page's lease
+      // lapsed mid-capture, or the request timed out): drop the local
+      // session and hand the surface back.
       if (body.error === 'agent_target_already_served' && msg.type === 'generate'
           && msg.id && msg.id === currentSessionId) {
         abandonSupersededGo(msg.id);
@@ -7767,10 +7768,10 @@
 
   function abandonSupersededGo(sessionId) {
     if (sessionId !== currentSessionId) return;
-    console.warn('[impeccable] Another page already served this agent target; clearing session ' + sessionId + '.');
+    console.warn('[impeccable] The helper already answered this agent target; clearing session ' + sessionId + '.');
     markSessionHandled();
     cleanup({ instantChrome: true });
-    showToast('Another tab already served this request, so this session was cleared.', 6000);
+    showToast('The helper already answered this request, so this session was cleared. Pick an element to start fresh.', 6000);
   }
 
   let abandonedForeignSessionId = null;

tests/live-agent-target.test.mjs

@@ -808,6 +808,35 @@ describe('POST /agent-target', { skip: ENGINE_BIN ? false : ENGINE_MISSING_MESSA
     }
   });
 
+  it('fences a generate event that lands after the request timed out, so no session the agent was never told about starts', async () => {
+    const tabA = await openSseClient(server, { clientId: 'tab-a' });
+    try {
+      await tabA.next((m) => m.type === 'connected');
+      const held = postJson(server, '/agent-target', {
+        token: server.token, selector: 'h1', action: 'bolder', count: 3,
+      });
+      const pushed = await tabA.next((m) => m.type === 'agent_target');
+      const claim = await (await postJson(server, '/agent-target-claim', {
+        token: server.token, targetId: pushed.targetId, clientId: 'tab-a', eligible: true,
+      })).json();
+      assert.equal(claim.granted, true);
+      const verdict = await (await held).json();
+      assert.equal(verdict.error, 'browser_timeout');
+      const late = await postJson(server, '/events', {
+        token: server.token, type: 'generate', id: 'eeeeeeee', action: 'bolder', count: 3, pageUrl: '/',
+        element: { tagName: 'h1', outerHTML: '<h1>Hero</h1>' },
+        agentTarget: { targetId: pushed.targetId, clientId: 'tab-a', result: { ok: true, matchCount: 1, sessionId: 'eeeeeeee', action: 'bolder', count: 3 } },
+      });
+      assert.equal(late.status, 409);
+      const body = await late.json();
+      assert.equal(body.error, 'agent_target_already_served');
+      assert.equal(body.sessionId, undefined);
+      assert.ok(!existsSync(join(tmp, '.impeccable/live/sessions/eeeeeeee.jsonl')), 'a fenced Go journals nothing');
+    } finally {
+      tabA.close();
+    }
+  });
+
   it('prefers busy over no_match, so the agent retries when the right page is mid-session', async () => {
     const tabA = await openSseClient(server, { clientId: 'tab-a' });
     const tabB = await openSseClient(server, { clientId: 'tab-b' });