Address review: a superseded Go never opens a second session

An overlay renews its lease right before Go, then captures and uploads
the element before its generate event leaves. When that outlasts the
lease and its result post is lost, a rescuer can claim the target and
Go, and the helper accepted both generate events: two sessions for one
request.

The generate envelope now carries this page's clientId, and the helper
refuses a generate event for a target that another page holds under a
live lease, or that was already answered with a different session
(`served_agent_targets`, recorded on every ok resolution): 409
`agent_target_already_served`, nothing journaled. The overlay treats
that refusal like a foreign session and hands the surface back. The
answering session's own event stays welcome, so the common path (result
post first, then the event) is unchanged.

Tests: two Rust integration cases (rival lease, answered elsewhere,
welcome for the serving session) and a Node protocol case, contract
pins for the envelope and the refusal handling, contract doc.

Written with AI assistance (Claude).

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

crates/cli/tests/agent_target.rs

@@ -485,7 +485,7 @@ fn agent_target_resolves_from_the_generate_event_when_the_result_never_lands() {
     let (status, ack) = post_json(s.port, "/events", serde_json::json!({
         "token": s.token, "type": "generate", "id": "aabbccdd", "action": "bolder", "count": 3, "pageUrl": "/",
         "element": { "tagName": "h1", "outerHTML": "<h1>Hero</h1>" },
-        "agentTarget": { "targetId": target_id, "result": result },
+        "agentTarget": { "targetId": target_id, "clientId": "tab-a", "result": result },
     }));
     assert_eq!(status, 200, "{ack}");
     let (_, verdict) = held.join().unwrap();
@@ -501,3 +501,61 @@ fn agent_target_resolves_from_the_generate_event_when_the_result_never_lands() {
     assert!(!journal.contains("agentTarget"), "{journal}");
     let _ = &mut b;
 }
+
+fn generate_event_for(s: &Server, target_id: &str, id: &str, client: &str) -> serde_json::Value {
+    serde_json::json!({
+        "token": s.token, "type": "generate", "id": id, "action": "bolder", "count": 3, "pageUrl": "/",
+        "element": { "tagName": "h1", "outerHTML": "<h1>Hero</h1>" },
+        "agentTarget": { "targetId": target_id, "clientId": client, "result": { "ok": true, "matchCount": 1, "sessionId": id, "action": "bolder", "count": 3 } },
+    })
+}
+
+#[test]
+fn agent_target_refuses_a_generate_event_from_a_superseded_claimant() {
+    let s = Server::start("superseded");
+    let mut a = Overlay::connect(s.port, &s.token, "tab-a");
+    let mut b = Overlay::connect(s.port, &s.token, "tab-b");
+    a.next(|m| m["type"] == "connected");
+    b.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));
+    // Tab A's lease (250ms) lapses while it is still capturing; tab B rescues.
+    std::thread::sleep(Duration::from_millis(300));
+    assert_eq!(s.claim(&target_id, "tab-b", true)["granted"], serde_json::json!(true));
+    // A's delayed event while B holds the lease: refused, nothing journaled.
+    let (status, body) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "aaaaaaaa", "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/aaaaaaaa.jsonl").exists());
+    // B's Go serves the request.
+    let (status, _) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "bbbbbbbb", "tab-b"));
+    assert_eq!(status, 200);
+    let (_, verdict) = held.join().unwrap();
+    assert_eq!(verdict["sessionId"], serde_json::json!("bbbbbbbb"), "{verdict}");
+    // A's event once the request was answered elsewhere: refused, naming
+    // the session that serves it.
+    let (status, body) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "aaaaaaa2", "tab-a"));
+    assert_eq!(status, 409, "{body}");
+    assert_eq!(body["sessionId"], serde_json::json!("bbbbbbbb"));
+    assert!(!s.dir.join(".impeccable/live/sessions/aaaaaaa2.jsonl").exists());
+    let _ = &mut b;
+}
+
+#[test]
+fn agent_target_welcomes_the_generate_event_of_the_session_that_answered() {
+    let s = Server::start("welcome");
+    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 result post lands first (the common path), then the event.
+    post_json(s.port, "/agent-target-result", serde_json::json!({ "token": s.token, "targetId": target_id, "ok": true, "sessionId": "cccccccc" }));
+    let (_, verdict) = held.join().unwrap();
+    assert_eq!(verdict["sessionId"], serde_json::json!("cccccccc"), "{verdict}");
+    let (status, body) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "cccccccc", "tab-a"));
+    assert_eq!(status, 200, "{body}");
+    assert!(s.dir.join(".impeccable/live/sessions/cccccccc.jsonl").exists());
+}

crates/live/src/live_server.rs

@@ -184,6 +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(),
             shutting_down: false,
             cleaned_up: false,
             log_tx,
@@ -1238,6 +1239,23 @@ fn handle_events_post(
             return;
         }
     }
+    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()) {
+            // 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.
+            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);
+            }
+            respond(stream, cors, json_res(409, body));
+            return;
+        }
+    }
     let missed = st.detect_missed_generation_completion(&msg_obj);
     if id_truthy {
         if let Err(e) = st.store.append_event(&msg) {

crates/live/src/server_state.rs

@@ -116,6 +116,10 @@ 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)>,
     pub last_poll_at: i64,
     pub timed_out_apply_ids: Vec<(String, TimedOutApply)>,
     pub next_poll_id: u64,
@@ -816,10 +820,55 @@ 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 _ = 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(
+        &self,
+        envelope: &Map<String, Value>,
+        session_id: Option<&str>,
+    ) -> Option<String> {
+        let target_id = envelope.get("targetId").and_then(Value::as_str)?;
+        let client_id = envelope
+            .get("clientId")
+            .and_then(Value::as_str)
+            .unwrap_or("");
+        if let Some((_, pending)) = self
+            .pending_agent_targets
+            .iter()
+            .find(|(k, _)| k == target_id)
+        {
+            return match &pending.owner {
+                Some(owner) if owner != client_id && pending.claimed_until > now_i64() => {
+                    Some(String::new())
+                }
+                _ => None,
+            };
+        }
+        self.served_agent_targets
+            .iter()
+            .rev()
+            .find(|(t, _)| t == target_id)
+            .filter(|(_, sid)| Some(sid.as_str()) != session_id)
+            .map(|(_, sid)| sid.clone())
+    }
+
     /// Every connected overlay has declined: answer busy now, not at the
     /// timeout. Judged against the connections of this moment, so it runs
     /// whenever a report lands and whenever an overlay leaves.

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. |
+| `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-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,6 +7736,14 @@
     }).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.
+      if (body.error === 'agent_target_already_served' && msg.type === 'generate'
+          && msg.id && msg.id === currentSessionId) {
+        abandonSupersededGo(msg.id);
+        return null;
+      }
       // The server refused to journal progress for a session it has never
       // seen: this browser is carrying state from another project or a
       // wiped store (two apps sharing a localhost port). Continuing to
@@ -7757,6 +7765,14 @@
     return sessionCreationGate.then(doSend);
   }
 
+  function abandonSupersededGo(sessionId) {
+    if (sessionId !== currentSessionId) return;
+    console.warn('[impeccable] Another page already served this agent target; clearing session ' + sessionId + '.');
+    markSessionHandled();
+    cleanup({ instantChrome: true });
+    showToast('Another tab already served this request, so this session was cleared.', 6000);
+  }
+
   let abandonedForeignSessionId = null;
   function abandonForeignSession(sessionId) {
     if (abandonedForeignSessionId === sessionId || sessionId !== currentSessionId) return;
@@ -8192,6 +8208,7 @@
       // as well as from the overlay's own result post.
       basePayload.agentTarget = {
         targetId: agentTargetForGo.targetId,
+        clientId: AGENT_TARGET_CLIENT_ID,
         result: {
           ok: true,
           matchCount: agentTargetForGo.matchCount,

tests/live-agent-target.test.mjs

@@ -10,7 +10,7 @@
 
 import { describe, it, before, after } from 'node:test';
 import assert from 'node:assert/strict';
-import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
 import { dirname, join } from 'node:path';
 import { tmpdir } from 'node:os';
 import { execFile, execFileSync, spawn } from 'node:child_process';
@@ -757,7 +757,7 @@ describe('POST /agent-target', { skip: ENGINE_BIN ? false : ENGINE_MISSING_MESSA
       const ack = await postJson(server, '/events', {
         token: server.token, type: 'generate', id: 'aabbccdd', action: 'bolder', count: 3, pageUrl: '/',
         element: { tagName: 'h1', outerHTML: '<h1>Hero</h1>' },
-        agentTarget: { targetId: pushed.targetId, result },
+        agentTarget: { targetId: pushed.targetId, clientId: 'tab-a', result },
       });
       assert.equal(ack.status, 200);
       const verdict = await (await held).json();
@@ -775,6 +775,39 @@ describe('POST /agent-target', { skip: ENGINE_BIN ? false : ENGINE_MISSING_MESSA
     }
   });
 
+  it('refuses a generate event for a target another session already answered, and welcomes that session\'s own', 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);
+      await postJson(server, '/agent-target-result', { token: server.token, targetId: pushed.targetId, ok: true, sessionId: 'cccccccc' });
+      assert.equal((await (await held).json()).sessionId, 'cccccccc');
+      const event = (id, clientId) => postJson(server, '/events', {
+        token: server.token, type: 'generate', id, action: 'bolder', count: 3, pageUrl: '/',
+        element: { tagName: 'h1', outerHTML: '<h1>Hero</h1>' },
+        agentTarget: { targetId: pushed.targetId, clientId, result: { ok: true, matchCount: 1, sessionId: id, action: 'bolder', count: 3 } },
+      });
+      // A superseded Go from another page: refused, naming the serving session, nothing journaled.
+      const refused = await event('dddddddd', 'tab-b');
+      assert.equal(refused.status, 409);
+      const body = await refused.json();
+      assert.equal(body.error, 'agent_target_already_served');
+      assert.equal(body.sessionId, 'cccccccc');
+      assert.ok(!existsSync(join(tmp, '.impeccable/live/sessions/dddddddd.jsonl')), 'a refused Go journals nothing');
+      // The answering session's own event is welcome.
+      assert.equal((await event('cccccccc', 'tab-a')).status, 200);
+    } 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' });

tests/live-browser-source.test.mjs

@@ -872,8 +872,13 @@ describe('live-browser source contracts', () => {
     );
     assert.match(
       SOURCE,
-      /if \(agentTargetForGo\) \{[\s\S]{0,600}?basePayload\.agentTarget = \{[\s\S]{0,300}?sessionId: currentSessionId/,
-      'handleGo attaches the agent target with the session it minted',
+      /if \(agentTargetForGo\) \{[\s\S]{0,600}?basePayload\.agentTarget = \{\s*targetId: agentTargetForGo\.targetId,\s*clientId: AGENT_TARGET_CLIENT_ID,[\s\S]{0,300}?sessionId: currentSessionId/,
+      'handleGo attaches the agent target with this page\'s client id and the session it minted, so the helper can tell a superseded Go from the serving one',
+    );
+    assert.match(
+      SOURCE,
+      /body\.error === 'agent_target_already_served' && msg\.type === 'generate'\s*&& msg\.id && msg\.id === currentSessionId\) \{\s*abandonSupersededGo\(msg\.id\);\s*return null;/,
+      'a Go the helper refused as already served drops this page\'s local session instead of leaving it generating for nothing',
     );
     assert.match(
       SOURCE,