diff --git a/crates/cli/tests/agent_target.rs b/crates/cli/tests/agent_target.rs index 1d4a670b4..5aac8037c 100644 --- a/crates/cli/tests/agent_target.rs +++ b/crates/cli/tests/agent_target.rs @@ -468,3 +468,36 @@ fn agent_target_late_overlay_first_no_match_extends_the_grace() { assert!(reported_at.elapsed() < Duration::from_millis(400)); let _ = &mut b; } + +#[test] +fn agent_target_resolves_from_the_generate_event_when_the_result_never_lands() { + let s = Server::start("event-backstop"); + 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 fires Go: its generate event names the target it serves. Its + // own result post never lands (the page reloaded right after Go). + let result = serde_json::json!({ "ok": true, "matchCount": 1, "sessionId": "aabbccdd", "action": "bolder", "count": 3, "element": { "tag": "h1" } }); + 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 }, + })); + assert_eq!(status, 200, "{ack}"); + let (_, verdict) = held.join().unwrap(); + assert_eq!(verdict["sessionId"], serde_json::json!("aabbccdd"), "{verdict}"); + assert_eq!(verdict["targetId"], serde_json::json!(target_id)); + // Nothing is left pending for a rescuer to take over with a second Go. + let late = s.claim(&target_id, "tab-b", true); + assert_eq!(late["granted"], serde_json::json!(false), "{late}"); + assert_eq!(late["pending"], serde_json::json!(false), "{late}"); + // The journal carries the event without the envelope. + let journal = std::fs::read_to_string(s.dir.join(".impeccable/live/sessions/aabbccdd.jsonl")).unwrap(); + assert!(journal.contains("generate"), "{journal}"); + assert!(!journal.contains("agentTarget"), "{journal}"); + let _ = &mut b; +} diff --git a/crates/live/src/live_server.rs b/crates/live/src/live_server.rs index 2b4bd7fab..0b04aa8b3 100644 --- a/crates/live/src/live_server.rs +++ b/crates/live/src/live_server.rs @@ -1183,7 +1183,19 @@ fn handle_events_post( respond(stream, cors, json_res(400, json!({ "error": error }))); return; } + // A generate event may name the agent target it serves. The helper + // resolves that request from the event as well as from + // /agent-target-result, so a page that dies between Go and its result + // cannot leave the request pending for a second Go elsewhere. The + // envelope never reaches the journal or the poller. + let mut msg = msg; let mut msg_obj = msg_obj; + let agent_target = if ty == "generate" { + msg_obj.remove("agentTarget"); + msg.as_object_mut().and_then(|o| o.remove("agentTarget")) + } else { + None + }; crate::server_state::strip_poller_owned_event_fields(&mut msg_obj); let mut st = lock(shared); if ty == "agent_phase" { @@ -1260,6 +1272,13 @@ fn handle_events_post( if ty != "checkpoint" && ty != "variant_mounted" && !orphaned_discard { st.enqueue_event(msg_obj); } + if let Some(Value::Object(envelope)) = agent_target { + if let (Some(Value::String(target_id)), Some(result @ Value::Object(_))) = + (envelope.get("targetId"), envelope.get("result")) + { + st.resolve_agent_target(target_id, result.clone()); + } + } drop(st); respond(stream, cors, json_res(200, json!({ "ok": true }))); } diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index e3b6b2b67..e7457f95b 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). | +| `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-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 276eb8b95..80659f4b5 100644 --- a/skill/scripts/live-browser.js +++ b/skill/scripts/live-browser.js @@ -7211,6 +7211,10 @@ // it, and only the tab that holds the lease can renew it. const AGENT_TARGET_CLIENT_ID = id8(); + // The agent target an agent-initiated Go is serving: set by + // actOnAgentTarget around its handleGo call, read once by handleGo. + let agentTargetForGo = null; + function claimAgentTarget(targetId, report) { return fetch('http://localhost:' + PORT + '/agent-target-claim?token=' + TOKEN, { method: 'POST', @@ -7446,7 +7450,14 @@ updateBarContent('configure'); const input = uiGetById(PREFIX + '-input'); if (input) input.value = msg.prompt || ''; + // The target rides on the generate event too: the helper resolves + // the request from whichever lands first, so a page that dies + // between Go and its result cannot leave the request pending for a + // second Go elsewhere. + const candidate = describeAgentTargetCandidate(el); + agentTargetForGo = { targetId: msg.targetId, matchCount: resolved.matchCount, action: msg.action, count: msg.count, element: candidate }; handleGo(); + agentTargetForGo = null; if (state === 'GENERATING' && currentSessionId) { reply({ ok: true, @@ -7454,7 +7465,7 @@ sessionId: currentSessionId, action: msg.action, count: msg.count, - element: describeAgentTargetCandidate(el), + element: candidate, }); } else { reply({ ok: false, error: 'go_failed', state }); @@ -8175,6 +8186,23 @@ }; if (snapshot.comments.length > 0) basePayload.comments = snapshot.comments; if (snapshot.strokes.length > 0) basePayload.strokes = snapshot.strokes; + if (agentTargetForGo) { + // An agent-initiated Go names the target it serves (see + // actOnAgentTarget): the helper resolves that request from this event + // as well as from the overlay's own result post. + basePayload.agentTarget = { + targetId: agentTargetForGo.targetId, + result: { + ok: true, + matchCount: agentTargetForGo.matchCount, + sessionId: currentSessionId, + action: agentTargetForGo.action, + count: agentTargetForGo.count, + element: agentTargetForGo.element, + }, + }; + agentTargetForGo = null; + } // Hide the interactive overlay so it doesn't linger during generation. hideAnnotOverlay(); diff --git a/tests/live-agent-target.test.mjs b/tests/live-agent-target.test.mjs index 11f4369f1..45eab63a8 100644 --- a/tests/live-agent-target.test.mjs +++ b/tests/live-agent-target.test.mjs @@ -739,6 +739,42 @@ describe('POST /agent-target', { skip: ENGINE_BIN ? false : ENGINE_MISSING_MESSA } }); + it('resolves the request from the generate event that serves it, so a page that dies before its result cannot leave it pending', async () => { + const tabA = await openSseClient(server, { clientId: 'tab-a' }); + const tabB = await openSseClient(server, { clientId: 'tab-b' }); + try { + await tabA.next((m) => m.type === 'connected'); + await tabB.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 result = { ok: true, matchCount: 1, sessionId: 'aabbccdd', action: 'bolder', count: 3, element: { tag: 'h1' } }; + 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 }, + }); + assert.equal(ack.status, 200); + const verdict = await (await held).json(); + assert.equal(verdict.ok, true); + assert.equal(verdict.sessionId, 'aabbccdd'); + const late = await (await postJson(server, '/agent-target-claim', { + token: server.token, targetId: pushed.targetId, clientId: 'tab-b', eligible: true, + })).json(); + assert.deepEqual(late, { ok: true, granted: false, pending: false }, 'nothing is left for a rescuer to serve twice'); + const journal = readFileSync(join(tmp, '.impeccable/live/sessions/aabbccdd.jsonl'), 'utf-8'); + assert.ok(!journal.includes('agentTarget'), 'the envelope never reaches the journal'); + } finally { + tabA.close(); + tabB.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 a306c3b9b..b1b220010 100644 --- a/tests/live-browser-source.test.mjs +++ b/tests/live-browser-source.test.mjs @@ -865,6 +865,16 @@ describe('live-browser source contracts', () => { /function watchAgentTargetResolution\(msg, lastError\) \{\s*if \(agentTargetOverlayGone\(\) \|\| agentTargetTaken\(msg\.targetId\)\) return;/, 'the late-mount watch stops once this page took the lease', ); + assert.match( + SOURCE, + /agentTargetForGo = \{ targetId: msg\.targetId, matchCount: resolved\.matchCount, action: msg\.action, count: msg\.count, element: candidate \};\s*handleGo\(\);\s*agentTargetForGo = null;/, + 'the target rides on the Go event it serves, so the helper resolves it even if this page dies before its result lands', + ); + 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', + ); assert.match( SOURCE, /if \(claim\.granted\) \{ noteAgentTarget\(msg\.targetId, 'acting'\); actOnAgentTarget\(msg\); return; \}/,