Address review: the server ends the resolution watch, and a reconnect re-participates

Two ways a page's word could go stale after the resolve-before-claim
change: an element that mounts later than the quick re-checks, and an
EventSource reconnect that did not overlap the old connection (the server
drops that page's word on the close, replays the target, and the replay
guard ignored it, so the roll call waited on a word that never came).

A decline's answer now carries pending, like a denied claim does, so a page
that could not resolve the target reports the miss after the quick
re-checks (the roll call can complete on the other overlays' words) and
keeps re-checking once a second for as long as the server says the request
is pending, claiming the moment the element mounts; the server drops the
stale report on an eligible claim and ends the watch by answering
pending:false once the request resolved or timed out. The overlay tracks
its participation per target: a replayed target is ignored only while this
page is acting on it, and is otherwise handled again, so a busy or
unresolvable page re-declines (idempotent) and an idle page claims.

Unit, protocol, and contract cases updated; the decline answers now say
whether the request is still pending.

AI-assisted: implemented and tested with Claude Code under maintainer
direction.

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

crates/cli/tests/agent_target.rs

@@ -274,8 +274,8 @@ fn agent_target_roll_call_answers_busy_once_every_overlay_declined() {
     let held = s.hold(serde_json::json!({}));
     let pushed = a.next(|m| m["type"] == "agent_target");
     let target_id = pushed["targetId"].as_str().unwrap().to_string();
-    assert_eq!(s.claim(&target_id, "tab-a", false), serde_json::json!({ "ok": true, "granted": false }));
-    assert_eq!(s.claim(&target_id, "tab-b", false), serde_json::json!({ "ok": true, "granted": false }));
+    assert_eq!(s.claim(&target_id, "tab-a", false), serde_json::json!({ "ok": true, "granted": false, "pending": true }), "the first decline leaves the request pending");
+    assert_eq!(s.claim(&target_id, "tab-b", false), serde_json::json!({ "ok": true, "granted": false, "pending": false }), "the last decline completes the roll call");
     let (_, verdict) = held.join().unwrap();
     assert_eq!(verdict["error"], serde_json::json!("busy"));
     assert_eq!(verdict["state"], serde_json::json!("CYCLING"));
@@ -370,7 +370,7 @@ fn agent_target_roll_call_counts_overlays_not_connections() {
     let target_id = a.next(|m| m["type"] == "agent_target")["targetId"].as_str().unwrap().to_string();
     // One overlay behind two connections reports busy once: that completes
     // the roll call instead of waiting on a "second" report until timeout.
-    assert_eq!(s.claim(&target_id, "tab-a", false), serde_json::json!({ "ok": true, "granted": false }));
+    assert_eq!(s.claim(&target_id, "tab-a", false), serde_json::json!({ "ok": true, "granted": false, "pending": false }), "one overlay behind two connections completes the roll call alone");
     let (_, verdict) = held.join().unwrap();
     assert_eq!(verdict["error"], serde_json::json!("busy"));
     assert!(started.elapsed() < Duration::from_millis(350), "the busy verdict did not wait for the timeout");
@@ -390,8 +390,8 @@ fn agent_target_answers_the_resolution_verdict_when_no_page_can_serve() {
     // Both idle pages lack the element: each declines with its resolution
     // verdict instead of claiming.
     let decline = |cid: &str, raw: u64| serde_json::json!({ "token": s.token, "targetId": target_id, "clientId": cid, "eligible": false, "state": "IDLE", "reason": "no_match", "result": { "ok": false, "error": "no_match", "selector": "h1", "matchCount": 0, "rawMatchCount": raw } });
-    assert_eq!(post_json(s.port, "/agent-target-claim", decline("tab-a", 0)).1, serde_json::json!({ "ok": true, "granted": false }));
-    assert_eq!(post_json(s.port, "/agent-target-claim", decline("tab-b", 2)).1, serde_json::json!({ "ok": true, "granted": false }));
+    assert_eq!(post_json(s.port, "/agent-target-claim", decline("tab-a", 0)).1, serde_json::json!({ "ok": true, "granted": false, "pending": true }));
+    assert_eq!(post_json(s.port, "/agent-target-claim", decline("tab-b", 2)).1, serde_json::json!({ "ok": true, "granted": false, "pending": false }), "the last decline completes the roll call");
     let (_, verdict) = held.join().unwrap();
     assert_eq!(verdict["error"], serde_json::json!("no_match"), "{verdict}");
     assert_eq!(verdict["ok"], serde_json::json!(false));
@@ -412,7 +412,7 @@ fn agent_target_prefers_busy_over_no_match_across_pages() {
     // The page that has the element is mid-session; the other page lacks it.
     // The agent should retry later, so busy outranks no_match.
     post_json(s.port, "/agent-target-claim", serde_json::json!({ "token": s.token, "targetId": target_id, "clientId": "tab-b", "eligible": false, "state": "IDLE", "reason": "no_match", "result": { "ok": false, "error": "no_match", "matchCount": 0, "rawMatchCount": 0 } }));
-    assert_eq!(s.claim(&target_id, "tab-a", false), serde_json::json!({ "ok": true, "granted": false }));
+    assert_eq!(s.claim(&target_id, "tab-a", false), serde_json::json!({ "ok": true, "granted": false, "pending": false }));
     let (_, verdict) = held.join().unwrap();
     assert_eq!(verdict["error"], serde_json::json!("busy"), "{verdict}");
     assert_eq!(verdict["reason"], serde_json::json!("session_active"));

crates/live/src/server_state.rs

@@ -894,7 +894,12 @@ impl ServerState {
                 pending.claimed_until = 0;
             }
             self.maybe_complete_agent_target_roll_call(target_id);
-            return json!({ "ok": true, "granted": false });
+            // `pending` tells a declining overlay whether to keep watching
+            // for a change of its word (an element that mounts late, a
+            // session that ends); false once the roll call or a result
+            // resolved the request.
+            let still_pending = self.pending_agent_targets.iter().any(|(k, _)| k == target_id);
+            return json!({ "ok": true, "granted": false, "pending": still_pending });
         }
         // An eligible claim is the client's latest word: drop any earlier
         // busy report, so a busy verdict only ever counts tabs still busy.

docs/CLI-CONTRACT.md

@@ -1481,7 +1481,7 @@ Binds `127.0.0.1:PORT`. CORS: if request has `Origin` and (origin is loopback ht
 | `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). |
-| `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}`, 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}`; otherwise 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}`. |
+| `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}`; otherwise 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` |
 
 Pending-event summary in `/status.pendingEvents[]`: `{id, type, leased:boolean, leaseUntil:number|null}` plus for `manual_edit_apply`: `pageUrl, chunk, repair, evidencePath, agentAction, manualApplySummary:{pageUrl, chunk, entryCount, opCount, files[]}`.

skill/scripts/live-browser.js

@@ -7236,6 +7236,7 @@
 
   function declineAgentTargetBusy(msg, busy) {
     busyDeclinedTargets.set(msg.targetId, msg);
+    noteAgentTarget(msg.targetId, 'declined');
     claimAgentTarget(msg.targetId, { eligible: false, state, reason: busy });
   }
 
@@ -7265,7 +7266,8 @@
     if (busy) { declineAgentTargetBusy(msg, busy); return; }
     if (declineAgentTargetUnresolvable(msg)) return;
     claimAgentTarget(msg.targetId, { eligible: true }).then((claim) => {
-      if (claim.granted) { actOnAgentTarget(msg); return; }
+      if (claim.granted) { noteAgentTarget(msg.targetId, 'acting'); actOnAgentTarget(msg); return; }
+      noteAgentTarget(msg.targetId, 'denied');
       if (!claim.pending) return;
       setTimeout(() => claimAndActOnAgentTarget(msg), AGENT_TARGET_RESCUE_RETRY_MS);
     });
@@ -7279,11 +7281,18 @@
     }
   }
 
-  // Targets this page already answered (claimed, declined, or acted on).
-  // The server replays pending targets to every connection that opens, and
-  // an EventSource reconnect opens one for a page that already heard the
-  // target, so a replay must not start a second claim or a second Go.
-  const agentTargetsSeen = [];
+  // This page's participation in each target it heard: 'acting' once a
+  // claim was granted (so a replay never starts a second Go), else the word
+  // it last gave. The server replays pending targets to every connection
+  // that opens. After a reconnect that overlapped the old connection the
+  // server still holds this page's word; after one that did not, it dropped
+  // the word on the close, so a replayed target is handled again: a busy or
+  // unresolvable page re-declines (idempotent), an idle page claims.
+  const agentTargetsSeen = new Map();
+  function noteAgentTarget(targetId, status) {
+    agentTargetsSeen.set(targetId, status);
+    if (agentTargetsSeen.size > 100) agentTargetsSeen.delete(agentTargetsSeen.keys().next().value);
+  }
 
   // Only a page that can resolve the target claims it. A tab whose page
   // lacks the element declines with its resolution verdict instead, so a
@@ -7298,6 +7307,13 @@
   // moment the element mounts, and only the last miss is reported. The
   // server's timeout still bounds the whole exchange.
   const AGENT_TARGET_RESOLVE_RETRY_MS = [300, 700, 1500];
+  // After the quick re-checks the page reports the miss (so the roll call
+  // can complete on the other overlays' words) and keeps re-checking at
+  // this cadence for as long as the server says the request is pending,
+  // claiming the moment the element mounts; the server drops the stale
+  // report on an eligible claim and ends the watch by answering
+  // pending:false once the request resolved or timed out.
+  const AGENT_TARGET_RESOLVE_WATCH_MS = 1000;
 
   function declineAgentTargetUnresolvable(msg) {
     const probe = resolveAgentTargetElement(msg);
@@ -7308,7 +7324,11 @@
 
   function retryAgentTargetResolution(msg, attempt, lastError) {
     if (attempt >= AGENT_TARGET_RESOLVE_RETRY_MS.length) {
-      claimAgentTarget(msg.targetId, { eligible: false, state, reason: 'no_match', result: lastError });
+      noteAgentTarget(msg.targetId, 'declined');
+      claimAgentTarget(msg.targetId, { eligible: false, state, reason: 'no_match', result: lastError }).then((answer) => {
+        if (!answer.pending) return;
+        setTimeout(() => watchAgentTargetResolution(msg, lastError), AGENT_TARGET_RESOLVE_WATCH_MS);
+      });
       return;
     }
     setTimeout(() => {
@@ -7321,11 +7341,24 @@
     }, AGENT_TARGET_RESOLVE_RETRY_MS[attempt]);
   }
 
+  function watchAgentTargetResolution(msg, lastError) {
+    if (agentTargetOverlayGone() || agentTargetsSeen.get(msg.targetId) === 'acting') return;
+    const busy = agentTargetBusyReason();
+    if (busy) { declineAgentTargetBusy(msg, busy); return; }
+    const probe = resolveAgentTargetElement(msg);
+    if (!probe.error) { claimAndActOnAgentTarget(msg); return; }
+    // Still unresolvable: re-decline (idempotent) and let the answer say
+    // whether to keep watching.
+    claimAgentTarget(msg.targetId, { eligible: false, state, reason: 'no_match', result: probe.error || lastError }).then((answer) => {
+      if (!answer.pending) return;
+      setTimeout(() => watchAgentTargetResolution(msg, lastError), AGENT_TARGET_RESOLVE_WATCH_MS);
+    });
+  }
+
   function handleAgentTarget(msg) {
     if (!msg || typeof msg.targetId !== 'string') return;
-    if (agentTargetsSeen.includes(msg.targetId)) return;
-    agentTargetsSeen.push(msg.targetId);
-    if (agentTargetsSeen.length > 100) agentTargetsSeen.shift();
+    if (agentTargetsSeen.get(msg.targetId) === 'acting') return;
+    noteAgentTarget(msg.targetId, 'heard');
     const busy = agentTargetBusyReason();
     if (busy) {
       // Roll call: a busy tab reports itself and never acts. The server

tests/live-agent-target.test.mjs

@@ -358,11 +358,11 @@ describe('POST /agent-target', { skip: ENGINE_BIN ? false : ENGINE_MISSING_MESSA
         token: server.token, selector: 'h1', action: 'bolder', count: 3,
       });
       const pushed = await tabA.next((m) => m.type === 'agent_target');
-      for (const clientId of ['tab-a', 'tab-b']) {
+      for (const [clientId, pending] of [['tab-a', true], ['tab-b', false]]) {
         const report = await (await postJson(server, '/agent-target-claim', {
           token: server.token, targetId: pushed.targetId, clientId, eligible: false, state: 'CYCLING', reason: 'session_active',
         })).json();
-        assert.deepEqual(report, { ok: true, granted: false });
+        assert.deepEqual(report, { ok: true, granted: false, pending }, 'a decline says whether the request is still pending');
       }
       const verdict = await (await held).json();
       assert.equal(verdict.error, 'busy');
@@ -654,12 +654,12 @@ describe('POST /agent-target', { skip: ENGINE_BIN ? false : ENGINE_MISSING_MESSA
         token: server.token, selector: 'h1', action: 'bolder', count: 3,
       });
       const pushed = await tabA.next((m) => m.type === 'agent_target');
-      for (const [clientId, raw] of [['tab-a', 0], ['tab-b', 3]]) {
+      for (const [clientId, raw, pending] of [['tab-a', 0, true], ['tab-b', 3, false]]) {
         const report = await (await postJson(server, '/agent-target-claim', {
           token: server.token, targetId: pushed.targetId, clientId, eligible: false, state: 'IDLE', reason: 'no_match',
           result: { ok: false, error: 'no_match', selector: 'h1', matchCount: 0, rawMatchCount: raw },
         })).json();
-        assert.deepEqual(report, { ok: true, granted: false });
+        assert.deepEqual(report, { ok: true, granted: false, pending });
       }
       const verdict = await (await held).json();
       assert.equal(verdict.error, 'no_match');

tests/live-browser-source.test.mjs

@@ -848,8 +848,13 @@ describe('live-browser source contracts', () => {
     );
     assert.match(
       SOURCE,
-      /function handleAgentTarget\(msg\) \{[\s\S]{0,120}?if \(agentTargetsSeen\.includes\(msg\.targetId\)\) return;/,
-      'a replayed target this page already handled must not start a second claim or Go',
+      /function handleAgentTarget\(msg\) \{[\s\S]{0,120}?if \(agentTargetsSeen\.get\(msg\.targetId\) === 'acting'\) return;/,
+      'a replayed target this page is acting on must not start a second claim or Go; any other replay is handled again',
+    );
+    assert.match(
+      SOURCE,
+      /if \(claim\.granted\) \{ noteAgentTarget\(msg\.targetId, 'acting'\); actOnAgentTarget\(msg\); return; \}/,
+      'a granted claim marks the target as acting before Go',
     );
     // A page that cannot resolve the target never claims it: a first-wins
     // claim would otherwise let the wrong page answer no_match for a target
@@ -871,8 +876,13 @@ describe('live-browser source contracts', () => {
     );
     assert.match(
       SOURCE,
-      /function retryAgentTargetResolution\(msg, attempt, lastError\) \{[\s\S]{0,200}?reason: 'no_match', result: lastError[\s\S]{0,600}?if \(!probe\.error\) \{ claimAndActOnAgentTarget\(msg\); return; \}/,
-      'the page claims the moment the element mounts, and reports only the last miss',
+      /function retryAgentTargetResolution\(msg, attempt, lastError\) \{[\s\S]{0,300}?reason: 'no_match', result: lastError[\s\S]{0,120}?if \(!answer\.pending\) return;[\s\S]{0,120}?watchAgentTargetResolution\(msg, lastError\)/,
+      'after the quick re-checks the page reports the miss and keeps watching while the server says the request is pending',
+    );
+    assert.match(
+      SOURCE,
+      /function watchAgentTargetResolution\(msg, lastError\) \{[\s\S]{0,400}?if \(!probe\.error\) \{ claimAndActOnAgentTarget\(msg\); return; \}[\s\S]{0,500}?if \(!answer\.pending\) return;/,
+      'a late mount turns into a claim, and the server ends the watch',
     );
     // The per-origin session cache must not let a tab on another page of
     // the app resume this page's session (it would sit in GENERATING for a
@@ -890,8 +900,8 @@ describe('live-browser source contracts', () => {
     );
     assert.equal(
       (SOURCE.match(/claimAndActOnAgentTarget\(msg\)/g) || []).length,
-      5,
-      'the first claim, the busy-to-idle re-claim, and the resolution re-check must share the rescue path (definition, three call sites, the retry)',
+      6,
+      'the first claim, the busy-to-idle re-claim, the resolution re-check, and the resolution watch must share the rescue path (definition, four call sites, the retry)',
     );
   });