diff --git a/crates/cli/tests/agent_target.rs b/crates/cli/tests/agent_target.rs index 5aac8037c..43fe008c0 100644 --- a/crates/cli/tests/agent_target.rs +++ b/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": "

Hero

" }, - "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": "

Hero

" }, + "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()); +} diff --git a/crates/live/src/live_server.rs b/crates/live/src/live_server.rs index 0b04aa8b3..bd5bf32ce 100644 --- a/crates/live/src/live_server.rs +++ b/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) { diff --git a/crates/live/src/server_state.rs b/crates/live/src/server_state.rs index d484447f5..168c1d5a5 100644 --- a/crates/live/src/server_state.rs +++ b/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, + session_id: Option<&str>, + ) -> Option { + 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. diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index e7457f95b..edba94c28 100644 --- a/docs/CLI-CONTRACT.md +++ b/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":}`, messages verbatim): `agent_target: selector is required`, `agent_target: selector too long` (>1000 chars), `agent_target: invalid action (valid: )`, `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` | diff --git a/skill/scripts/live-browser.js b/skill/scripts/live-browser.js index 80659f4b5..0ecfee894 100644 --- a/skill/scripts/live-browser.js +++ b/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, diff --git a/tests/live-agent-target.test.mjs b/tests/live-agent-target.test.mjs index 45eab63a8..b9e37a099 100644 --- a/tests/live-agent-target.test.mjs +++ b/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: '

Hero

' }, - 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: '

Hero

' }, + 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' }); diff --git a/tests/live-browser-source.test.mjs b/tests/live-browser-source.test.mjs index b1b220010..cd97352f4 100644 --- a/tests/live-browser-source.test.mjs +++ 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,