Address review: a Go for a target the helper does not hold is refused

The bounded record of answered targets evicted its oldest entry, and a
generate event naming an unrecognized target was admitted, so a Go
delayed past enough later resolutions could still open a session for
a request the CLI had reported as failed.

The admission rule is now positive: a generate event naming an agent
target is welcome only while that target is pending without a rival
lease, or when it comes from the session that answered it. Unknown
targets, evicted or never issued, are refused like any other superseded
Go, so eviction can never reopen a request. The record keeps 256
entries for the answering session's sake.

Tests: a Rust integration case and a Node protocol case (an envelope
naming an unheld target is refused and journals nothing; the same event
without an envelope is an ordinary Go); contract doc updated.

Written with AI assistance (Claude).

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

crates/cli/tests/agent_target.rs

@@ -582,3 +582,20 @@ fn agent_target_fences_a_generate_event_that_lands_after_the_timeout() {
     assert!(body.get("sessionId").is_none(), "{body}");
     assert!(!s.dir.join(".impeccable/live/sessions/eeeeeeee.jsonl").exists());
 }
+
+#[test]
+fn agent_target_refuses_a_generate_event_for_a_target_it_never_held() {
+    // Unknown means refused: a target this helper never issued, or one
+    // evicted from its bounded record, can never be reopened by a late Go.
+    let s = Server::start("unknown-target");
+    let (status, body) = post_json(s.port, "/events", generate_event_for(&s, "0badf00d", "ffffffff", "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/ffffffff.jsonl").exists());
+    // Without an envelope the same event is an ordinary Go.
+    let mut plain = generate_event_for(&s, "0badf00d", "ffffffff", "tab-a");
+    plain.as_object_mut().unwrap().remove("agentTarget");
+    let (status, _) = post_json(s.port, "/events", plain);
+    assert_eq!(status, 200);
+}

crates/live/src/server_state.rs

@@ -124,10 +124,11 @@ pub struct ServerState {
     pub pending_agent_targets: Vec<(String, AgentTargetPending)>,
     pub next_agent_target_timer_gen: u64,
     /// 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.
+    /// the session that answered it when the verdict carried one. A
+    /// generate event naming a target is welcome only while that target is
+    /// pending without a rival lease, or when it comes from the session
+    /// that answered it; anything else, including a target this record no
+    /// longer holds, is refused, so eviction can never reopen a request.
     pub resolved_agent_targets: Vec<(String, Option<String>)>,
     pub last_poll_at: i64,
     pub timed_out_apply_ids: Vec<(String, TimedOutApply)>,
@@ -839,7 +840,7 @@ impl ServerState {
         };
         self.resolved_agent_targets
             .push((target_id.to_string(), session));
-        if self.resolved_agent_targets.len() > 64 {
+        if self.resolved_agent_targets.len() > 256 {
             self.resolved_agent_targets.remove(0);
         }
         let _ = pending.tx.send(result);
@@ -849,11 +850,13 @@ impl ServerState {
     /// 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
+    /// it (this page's lease lapsed while it was capturing); 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.
+    /// timeout or a failure verdict the CLI has already reported); or the
+    /// helper neither holds nor remembers the target (never issued here, or
+    /// long since evicted from the bounded record). None only when the
+    /// event is welcome: a pending target without a rival, or the
+    /// answering session's own event.
     pub fn agent_target_refusal(
         &self,
         envelope: &Map<String, Value>,
@@ -876,11 +879,14 @@ impl ServerState {
                 _ => None,
             };
         }
-        let (_, answered_by) = self
+        let Some((_, answered_by)) = self
             .resolved_agent_targets
             .iter()
             .rev()
-            .find(|(t, _)| t == target_id)?;
+            .find(|(t, _)| t == target_id)
+        else {
+            return Some(AgentTargetRefusal { session_id: None });
+        };
         if answered_by.as_deref() == session_id && session_id.is_some() {
             return None;
         }

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 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-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), or that the helper neither holds nor remembers (never issued by it, or evicted from its bounded record of answered targets), 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` |
 

tests/live-agent-target.test.mjs

@@ -837,6 +837,20 @@ describe('POST /agent-target', { skip: ENGINE_BIN ? false : ENGINE_MISSING_MESSA
     }
   });
 
+  it('refuses a generate event naming a target the helper never held, so eviction can never reopen a request', async () => {
+    const event = (agentTarget) => postJson(server, '/events', {
+      token: server.token, type: 'generate', id: 'ffffffff', action: 'bolder', count: 3, pageUrl: '/',
+      element: { tagName: 'h1', outerHTML: '<h1>Hero</h1>' },
+      ...(agentTarget ? { agentTarget } : {}),
+    });
+    const refused = await event({ targetId: '0badf00d', clientId: 'tab-a', result: { ok: true, sessionId: 'ffffffff' } });
+    assert.equal(refused.status, 409);
+    assert.equal((await refused.json()).error, 'agent_target_already_served');
+    assert.ok(!existsSync(join(tmp, '.impeccable/live/sessions/ffffffff.jsonl')), 'nothing journaled');
+    // Without an envelope the same event is an ordinary Go.
+    assert.equal((await event(null)).status, 200);
+  });
+
   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' });