live-server: a departed overlay's report is not a roll-call word

A claim carrying a clientId no connection holds any more (the page
unloaded between the broadcast and the claim landing) was still recorded,
so a departed tab's busy or no_match report could complete the roll call,
or set its verdict, against the overlays that remain. Such a report is
now answered `{granted:false, pending:true}` and not recorded, while any
connection that sent no clientId keeps every id counted as connected.
Eligible claims are left as they were: a lease a departed page holds
lapses and a rescuer takes it, and refusing them would also refuse the
renew a live overlay sends inside an EventSource reconnect gap.

Written with AI assistance (Claude).

Co-Authored-By: Claude Fable 5.1 <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.1
parent 2f4d4fb4fc
commit 2d2009d7c1
3 changed files with 59 additions and 2 deletions
+35
View File
@@ -387,6 +387,41 @@ fn agent_target_roll_call_counts_overlays_not_connections() {
let _ = &mut a2;
}
#[test]
fn agent_target_ignores_the_word_of_a_departed_overlay() {
let s = Server::start("departed");
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");
// B's page goes away; wait until the helper has seen it leave.
drop(b);
let mut alone = false;
let mut last = serde_json::Value::Null;
for _ in 0..40 {
let (_, body) = http(s.port, "GET", &format!("/status?token={}", s.token), None);
let status: serde_json::Value = serde_json::from_str(&body).unwrap_or(serde_json::Value::Null);
if status["connectedClients"] == serde_json::json!(1) {
alone = true;
break;
}
last = status;
std::thread::sleep(Duration::from_millis(15));
}
assert!(alone, "the helper noticed B leave: {last}");
let held = s.hold(serde_json::json!({}));
let target_id = a.next(|m| m["type"] == "agent_target")["targetId"].as_str().unwrap().to_string();
// A busy report under B's id is nobody's word now: it must neither
// complete the roll call (A has not spoken) nor set its verdict.
assert_eq!(s.claim(&target_id, "tab-b", false), serde_json::json!({ "ok": true, "granted": false, "pending": true }));
std::thread::sleep(Duration::from_millis(60));
assert!(!held.is_finished(), "a departed overlay's busy report answered the request");
assert_eq!(s.claim(&target_id, "tab-a", true)["granted"], serde_json::json!(true));
post_json(s.port, "/agent-target-result", serde_json::json!({ "token": s.token, "targetId": target_id, "clientId": "tab-a", "ok": true, "sessionId": "aabbccdd" }));
let (_, verdict) = held.join().unwrap();
assert_eq!(verdict["sessionId"], serde_json::json!("aabbccdd"));
}
#[test]
fn agent_target_answers_the_resolution_verdict_when_no_page_can_serve() {
let s = Server::start("no-match");
+22
View File
@@ -740,6 +740,16 @@ impl ServerState {
}
}
/// Whether a connection carries this overlay's clientId. True as well
/// while any connection sent none (an older overlay build): that
/// overlay cannot be told apart from the id in hand.
fn overlay_connected(&self, client_id: &str) -> bool {
self.sse_clients.iter().any(|c| match c.agent_client_id.as_deref() {
Some(cid) => cid == client_id,
None => true,
})
}
/// Connected overlays for a roll call: one per distinct clientId, plus
/// every connection that sent none (an older overlay build), so a
/// reconnect's momentary duplicate connection never waits on a second
@@ -1063,6 +1073,7 @@ impl ServerState {
) -> Value {
let lease_ms = self.agent_target_lease_ms();
let now = now_i64();
let reporter_connected = eligible || self.overlay_connected(client_id);
let Some((_, pending)) = self
.pending_agent_targets
.iter_mut()
@@ -1071,6 +1082,17 @@ impl ServerState {
return json!({ "ok": true, "granted": false, "pending": false });
};
if !eligible {
// A report under an id no connection carries any more (the page
// unloaded between the broadcast and this claim landing) is not
// a participant's word: recorded, it could complete the roll
// call, or set its verdict, against the overlays that remain.
// An eligible claim is left alone: a lease a departed page holds
// lapses and a rescuer takes it, while refusing it would also
// refuse the renew a live overlay sends inside an EventSource
// reconnect gap, whose Go is still welcome.
if !reporter_connected {
return json!({ "ok": true, "granted": false, "pending": true });
}
let reason_is_no_match = reason.as_str() == Some("no_match");
// Only an overlay's first no_match word extends the grace: its
// re-reports while watching must not keep the roll call open.
+2 -2
View File
@@ -1482,7 +1482,7 @@ Binds `127.0.0.1:PORT`. CORS: if request has `Origin` and (origin is loopback ht
| `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` / `agent_target: hideLiveBar 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` and `clientId` (non-empty strings) required else 400 (`agent_target_result: missing targetId` / `agent_target_result: missing clientId`); while the target is pending, only its lease holder's `clientId` may answer: another overlay gets 409 `{"error":"agent_target_result: not the holder", reason:'not_holder'|'unclaimed', targetId}` and the request stays pending. Otherwise `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. An accepted generate event carrying the envelope is journaled and queued with `origin: "agent"` (the overlay restores its lane chrome from it); its `_instructions` are the same planning steps a user's Go gets. The envelope also carries `clientId`: a generate event from a page that is not the pending target's holder (another page holds the lease, or held it last, or nobody claimed it; a lapsed lease still belongs to the page that held it last until a rescuer claims), or naming a target 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 holder's own Go and the answering session's own event are welcome. |
| `POST /live-bar` | 401 / 400 Invalid JSON | `hidden` (boolean) required else 400 `{"error":"live_bar: hidden must be a boolean"}`. Sets the helper-wide bar preference; on a change broadcasts `{type:'live_bar', hidden}` to every SSE client. `GET /status` and the SSE `connected` frame carry it as `hideLiveBar`. Answers `{ok:true, hidden}`. |
| `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}`. |
| `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` (a report under a `clientId` no connection carries any more, its page having unloaded before the claim landed, is not recorded and answers `{ok:true, granted:false, pending:true}`; while any connection sent no `clientId`, every id counts as connected; 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` |
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[]}`.
@@ -1804,7 +1804,7 @@ Conventions: every script's "run directly" guard is `process.argv[1]` ending wit
- **Invoked from**: `skill/reference/generate.md` (the `generate` command), as the lane's one start command after the agent opened the page in its harness's own browser: `impeccable live-generate --target src/App.jsx --dev-url http://127.0.0.1:5173/ --selector ".pricing-grid" --action bolder --count 3 --boot --wait-for-browser 60000` (`--open` only on a harness with no browser tool; without `--dev-url` and without a wait, the verdict is `browser_needed` and the agent comes back with the page open).
- **Args**: `--selector <css>` (required), `--text <snippet>`, `--index <n>` (1-based), `--action <name>` (default `impeccable`), `--count <n>` (default 3, 1-8), `--prompt <text>`, `--dry-run`, `--wait-for-browser <ms>`, `--no-live-bar` (body `hideLiveBar:true`: the helper sets its lifetime-wide `hideLiveBar` preference, broadcasts `{type:'live_bar', hidden:true}` to every connected overlay before the target goes out, and answers `hideLiveBar:true` on every later `connected` frame; the overlay hides its global bar accordingly and skips its "No PRODUCT.md found" connect notice, the variant controls still show, and only the helper stopping ends it), `--target <path>` (consumed by `enterLiveRoot`, and the boot's --target under `--boot`), `--boot` (run `live --allow-missing-context --dev-url --no-live-bar [--target]` in-process from the caller's cwd first, reusing a running helper; implies `hideLiveBar:true` on the target; the boot's `devUrl, pageFiles, projectRoot, targetPath, liveBarHidden, contextMissing, contextNote, hasProduct, product, productPath, hasDesign, design, designPath, hasSurfaceBrief, surfaceBrief, surfaceBriefPath` ride along as `boot`; a refusing boot is printed as its own payload plus `ok:false`, `bootError:<its error>` and `_instructions`, exit 1; an unparseable boot → `boot_failed` (+`exitCode`, `detail`)), `--open` (ignored, with `openIgnored:'harness browser'` on the verdict and an `_instructions` prefix saying so, when the provider id is `cursor` or `claude-code` and neither `IMPECCABLE_BROWSER` nor the config's `browser` names a browser: a harness with its own browser never gets a second window from this verb, and the generic `BROWSER` variable is not that choice; otherwise, when `/status` reports no connected client: the dev URL is the boot's `devUrl` or a fresh `dev_url::probe`; none → `no_dev_server`, exit 1; else the URL is opened without waiting for the browser (`IMPECCABLE_BROWSER`, then `browser` in `.impeccable/config.local.json` / `.impeccable/config.json` at the app root, then `BROWSER`, then `open` / `xdg-open` / `cmd /c start`; a value with a path separator runs as a program with the URL as its argument, on macOS any other value is `open -a <name>`), recorded as `opened:{url, via}`, and `--wait-for-browser` defaults to 60000; a launch failure → `browser_open_failed` (+`url`, `detail`)), `--allow-missing-context` (accepted and ignored), `--dev-url [<url>]` (bare: ignored, the boot's own flag; with a value: the dev server the caller already knows, put first in the boot's probe list (`IMPECCABLE_DEV_URL_CANDIDATES` still wins when set) and in this verb's own probe, and reported as `devUrl` with `devUrlVerified:false` when no probe confirmed the tag on it), `--help`. A flag without a value → stdout `{"ok":false,"error":"missing_flag_value","flag":"--x"}`, exit 1.
- **Env**: `IMPECCABLE_SELF` (how the boot and poll verbs are spelled in `_instructions`).
- **Behavior**: `enterLiveRoot`; local verdicts first, each pretty-printed JSON on stdout with `_instructions`, exit 1: `selector_required`, `invalid_action` (+`action`, `validActions`), `invalid_count` (+`count`), `invalid_index` (+`index`), `invalid_wait` (+`wait`); no `server.json` (or one without port/token) → `server_not_running`. With `--wait-for-browser`, `GET /status` once a second until `connectedClients > 0` or the budget ends (`no_browser_connected` + `waitedMs`); while it waits, the dev URL it knows (the one it opened, else the boot's `devUrl`, else the probe or the caller's hint) gets a TCP connect every third tick, and two misses in a row end the wait early with `dev_server_gone` (+`devUrl`, `waitedMs`, `harness`) whose `_instructions` name the harness's way to start the dev script and rerun with `--dev-url`; an unanswered `/status``server_unreachable`. Then `POST /agent-target` with `{token, selector, action, count, text?, index?, prompt?, dryRun?}` under a 20 s client cap: a transport timeout → `request_timeout` (+`detail`, browser_timeout instructions), any other transport failure → `server_unreachable` (+`detail`); a non-2xx answer → `{ok:false, error:<body.error or http_<status>>, ...body}`; an unparseable body → `bad_server_response` (+`status`). Before the target goes out, with no page connected (`/status` `connectedClients` 0), no `--open`, no `--wait-for-browser`, and no `--dry-run`: `browser_needed` (`devUrl` from the boot's probe, else a probe led by the hint, else the hint unverified; `devUrlVerified`; `harness`, the provider id) or `no_dev_server` when no URL is known, exit 1. Its `_instructions` name the harness's own browser and never a second one: `cursor``browser_navigate` (reuse the tab on that origin); `claude-code` → the Browser pane (`navigate` the tab already on that origin, `tabs_context`, `preview_start` with the URL when the pane is closed); `codex` → rerun with `--open` or give the user the URL; others → the harness browser tool, else `--open` or the user; then rerun with `--wait-for-browser 60000`. `no_dev_server` names where this harness starts a server (`claude-code`: `preview_start` or the dev script; others: the dev script in a background terminal) and asks for `--dev-url <url>` on the rerun. Every verdict from the boot on carries `harness`. A started session (`ok:true`, not a dry run) then collects its own generate event: `GET /poll?types=generate&id=<sessionId>` in ≤5 s slices for up to 20 s, leased exactly as a poll leases it (the preflight scaffold runs on lease), printed as `event` with locally generated `_instructions` (the same planning steps `live-poll` would print for a user's Go), or `event:null` when it did not arrive. A 2xx answer is printed as received plus `boot`/`opened` when those ran, plus `_instructions` for `ok` (dry run; started session with `event`, pointing at the edit and at `live-poll --reply <id> done --file <path> --then-poll`; started session without it, pointing at `live-poll` first), `no_dev_server`, `browser_open_failed`, `no_browser_connected` (a variant when `opened` is present), `browser_timeout`, `invalid_selector`, `no_match` (wording depends on `rawMatchCount`), `ambiguous`, `index_out_of_range`, `busy`, `go_failed`, `server_stopping`; exit 0 when `ok:true`, else 1. `_instructions` are regenerated locally from the verdict, never taken from the wire.
- **Behavior**: `enterLiveRoot`; local verdicts first, each pretty-printed JSON on stdout with `_instructions`, exit 1: `selector_required`, `invalid_action` (+`action`, `validActions`), `invalid_count` (+`count`), `invalid_index` (+`index`), `invalid_wait` (+`wait`); no `server.json` (or one without port/token) → `server_not_running`. With `--wait-for-browser`, `GET /status` once a second until `connectedClients > 0` or the budget ends (`no_browser_connected` + `waitedMs`); while it waits, the dev URL it knows (the one it opened, else the boot's `devUrl`, else the probe or the caller's hint) gets a TCP connect every third tick (http or https alike, on every address its host resolves to), and two misses in a row end the wait early with `dev_server_gone` (+`devUrl`, `waitedMs`, `harness`) whose `_instructions` name the harness's way to start the dev script and rerun with `--dev-url`; an unanswered `/status``server_unreachable`. Then `POST /agent-target` with `{token, selector, action, count, text?, index?, prompt?, dryRun?}` under a 20 s client cap: a transport timeout → `request_timeout` (+`detail`, browser_timeout instructions), any other transport failure → `server_unreachable` (+`detail`); a non-2xx answer → `{ok:false, error:<body.error or http_<status>>, ...body}`; an unparseable body → `bad_server_response` (+`status`). Before the target goes out, with no page connected (`/status` `connectedClients` 0), no `--open`, no `--wait-for-browser`, and no `--dry-run`: `browser_needed` (`devUrl` from the boot's probe, else a probe led by the hint, else the hint unverified; `devUrlVerified`; `harness`, the provider id) or `no_dev_server` when no URL is known, exit 1. Its `_instructions` name the harness's own browser and never a second one: `cursor``browser_navigate` (reuse the tab on that origin); `claude-code` → the Browser pane (`navigate` the tab already on that origin, `tabs_context`, `preview_start` with the URL when the pane is closed); `codex` → rerun with `--open` or give the user the URL; others → the harness browser tool, else `--open` or the user; then rerun with `--wait-for-browser 60000`. `no_dev_server` names where this harness starts a server (`claude-code`: `preview_start` or the dev script; others: the dev script in a background terminal) and asks for `--dev-url <url>` on the rerun. Every verdict from the boot on carries `harness`. A started session (`ok:true`, not a dry run) then collects its own generate event: `GET /poll?types=generate&id=<sessionId>` in ≤5 s slices for up to 20 s, leased exactly as a poll leases it (the preflight scaffold runs on lease), printed as `event` with locally generated `_instructions` (the same planning steps `live-poll` would print for a user's Go), or `event:null` when it did not arrive. A 2xx answer is printed as received plus `boot`/`opened` when those ran, plus `_instructions` for `ok` (dry run; started session with `event`, pointing at the edit and at `live-poll --reply <id> done --file <path> --then-poll`; started session without it, pointing at `live-poll` first), `no_dev_server`, `browser_open_failed`, `no_browser_connected` (a variant when `opened` is present), `browser_timeout`, `invalid_selector`, `no_match` (wording depends on `rawMatchCount`), `ambiguous`, `index_out_of_range`, `busy`, `go_failed`, `server_stopping`; exit 0 when `ok:true`, else 1. `_instructions` are regenerated locally from the verdict, never taken from the wire.
- **Tests**: `tests/oracle/cases/live-generate.mjs` (local verdicts, no-browser), `tests/live-agent-target.test.mjs` (protocol matrix against the binary), `crates/cli/tests/agent_target.rs`, `tests/live-e2e.test.mjs` (`agentTargetScenario`).
#### `live-commit-manual-edits.mjs` -> `impeccable commit-manual-edits`