Address review: the generate event resolves the agent target it serves

A winning overlay could reload after handleGo() minted a session but
before its result post landed. The close released its lease, the
server replayed the still-pending target, and another tab (or the
reloaded page, once it abandoned the unknown session) could claim it
and fire a second Go for a request that already had a session.

The overlay now names the target on the generate event it fires for it
(`agentTarget: {targetId, result}`, the same result it posts), and the
helper resolves the pending request the moment that event is accepted,
stripping the envelope before journaling. Whichever of the event and
the result post lands first answers; a page that dies between Go and
its result cannot leave the request pending, and a request whose event
never reached the helper is served exactly once by the rescuer.

Tests: a Rust integration case and a Node protocol case (claim, Go
event without a result post, verdict carries the session, a late claim
finds nothing pending, the journal carries no envelope), contract pins
for the handoff, and the contract doc.

Written with AI assistance (Claude).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Abdul Wahab
2026-09-15 05:45:49 +05:00
committed by Abdul Wahab
co-authored by Claude Fable 5
parent 335945525d
commit b5210471fb
6 changed files with 128 additions and 2 deletions
+33
View File
@@ -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": "<h1>Hero</h1>" },
"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;
}
+19
View File
@@ -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 })));
}
+1 -1
View File
@@ -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). |
| `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` |
+29 -1
View File
@@ -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();
+36
View File
@@ -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: '<h1>Hero</h1>' },
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' });
+10
View File
@@ -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; \}/,