review: holder-only generate events, wrapper states and breakpoints in the bake

A generate event may open a session only from the page that holds the
target's lease, or held it last while its lease lapsed or its page went
away; a page that never claimed, or a lapsed holder once a rescuer has
claimed, is refused as before. The pending target remembers its last
holder for that.

The bake now lands a state written on the wrapper (`:scope:hover > .x`,
`:scope[open] > .x`) on the element that takes the wrapper's place, and
rewrites Astro's prefixed rules wherever they sit, `@media` and
`@supports` blocks at the top level included, instead of dropping the
variant's breakpoints.

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 899b7afcc9
commit df4740bad9
4 changed files with 175 additions and 77 deletions
+29
View File
@@ -552,6 +552,35 @@ fn agent_target_refuses_a_generate_event_from_a_superseded_claimant() {
let _ = &mut b;
}
#[test]
fn agent_target_refuses_a_generate_event_from_a_page_that_never_held_the_lease() {
let s = Server::start("stranger");
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();
// Before any claim, no page may open the session from an event.
let (status, body) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "cccccc01", "tab-b"));
assert_eq!(status, 409, "{body}");
assert!(!s.dir.join(".impeccable/live/sessions/cccccc01.jsonl").exists());
// A holds the lease; it lapses (250ms) with no rescuer. A stranger's
// event is still refused: a lapsed lease belongs to A until a rescuer
// claims.
assert_eq!(s.claim(&target_id, "tab-a", true)["granted"], serde_json::json!(true));
std::thread::sleep(Duration::from_millis(300));
let (status, body) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "cccccc02", "tab-b"));
assert_eq!(status, 409, "{body}");
assert!(!s.dir.join(".impeccable/live/sessions/cccccc02.jsonl").exists());
// A's own late Go is welcome and answers the request.
let (status, body) = post_json(s.port, "/events", generate_event_for(&s, &target_id, "cccccc03", "tab-a"));
assert_eq!(status, 200, "{body}");
let (_, verdict) = held.join().unwrap();
assert_eq!(verdict["sessionId"], serde_json::json!("cccccc03"), "{verdict}");
let _ = &mut b;
}
#[test]
fn agent_target_welcomes_the_generate_event_of_the_session_that_answered() {
let s = Server::start("welcome");
+127 -66
View File
@@ -88,26 +88,63 @@ fn is_css_ident(s: &str) -> bool {
&& !s.starts_with(|c: char| c.is_ascii_digit())
}
/// The first compound selector of `s` and what follows it (the following
/// combinator or whitespace included), honouring brackets, parens, and
/// quotes. `(s, "")` when there is no combinator.
fn split_first_compound(s: &str) -> (String, String) {
let chars: Vec<char> = s.chars().collect();
let mut depth = 0i64;
let mut quote: Option<char> = None;
let mut i = 0;
while i < chars.len() {
let c = chars[i];
if let Some(q) = quote {
if c == '\\' {
i += 1;
} else if c == q {
quote = None;
}
} else if c == '"' || c == '\'' {
quote = Some(c);
} else if c == '[' || c == '(' {
depth += 1;
} else if c == ']' || c == ')' {
depth -= 1;
} else if depth == 0 && (c.is_whitespace() || c == '>' || c == '+' || c == '~') {
break;
}
i += 1;
}
(chars[..i].iter().collect(), chars[i..].iter().collect())
}
/// One selector out of a `:scope` (or `[data-impeccable-variant="N"]`)
/// prefixed rule, anchored on the element. Err when `:scope` survives.
/// prefixed rule, anchored on the element. A state on the wrapper
/// (`:scope:hover`, `:scope[open]`) lands on the element, which is the
/// wrapper's only child and takes its place after the unwrap. Err when
/// `:scope` survives or the rewrite has no meaning.
pub fn rewrite_selector(selector: &str, anchor: &str) -> Result<String, String> {
let s = trim(selector).to_string();
let s = VARIANT_PREFIX_RE.replace(&s, ":scope").into_owned();
let out = if let Some(rest) = s.strip_prefix(":scope") {
let rest_trim = rest.trim_start();
if rest_trim.is_empty() {
anchor.to_string()
} else if let Some(child) = rest_trim.strip_prefix('>') {
// `:scope > .x`: the variant div's child is the element itself.
child.trim_start().to_string()
} else if rest_trim.starts_with(['+', '~']) {
// Pseudo-classes and attribute selectors written on :scope itself.
let (state, after) = split_first_compound(rest);
let after_trim = after.trim_start();
if after_trim.is_empty() {
format!("{}{}", anchor, state)
} else if let Some(child) = after_trim.strip_prefix('>') {
// `:scope > .x`, `:scope:hover > .x`: the wrapper's child is the
// element itself, so the wrapper's state is the element's.
let (first, remainder) = split_first_compound(child.trim_start());
if first.is_empty() {
return Err(format!("selector has no child after :scope: {}", selector));
}
format!("{}{}{}", first, state, remainder)
} else if after_trim.starts_with(['+', '~']) {
return Err(format!("sibling combinator on :scope has no meaning after unwrap: {}", selector));
} else if rest.starts_with(char::is_whitespace) {
// `:scope .x`: a descendant of the element.
format!("{} {}", anchor, rest_trim)
} else {
// `:scope:hover > .x`, `:scope[data-x] .y`: the element with that state.
format!("{}{}", anchor, rest)
// `:scope .x`, `:scope:hover .x`: a descendant of the element.
format!("{}{} {}", anchor, state, after_trim)
}
} else {
s
@@ -157,82 +194,78 @@ fn rewrite_nodes(nodes: &[CssNode], anchor: &str, out: &mut Vec<CssNode>, rules:
Ok(())
}
/// The accepted variant's rules out of the whole preview stylesheet: its
/// `@scope ([data-impeccable-variant="N"])` block rewritten and flattened,
/// global at-rules (`@keyframes`, `@font-face`) kept, the other variants'
/// blocks dropped.
pub fn extract_variant_css(css: &str, variant_num: &str, anchor: &str) -> Result<(String, usize), String> {
let nodes = parse_stylesheet(css);
let mut kept: Vec<CssNode> = Vec::new();
let mut rules = 0usize;
for node in &nodes {
/// A rule outside any `@scope`: Astro's global-prefixed mode writes
/// `[data-impeccable-variant="N"] > .x`. Ours are rewritten, other
/// variants' are dropped (None), plain rules are kept as they are.
fn global_rule(prelude: &str, body: &str, variant_num: &str, anchor: &str) -> Result<Option<CssNode>, String> {
let selectors = split_selector_list(prelude);
let mine: Vec<String> = selectors
.iter()
.filter(|sel| SCOPE_PRELUDE_RE.captures(sel).map(|c| &c[1] == variant_num).unwrap_or(false))
.cloned()
.collect();
if mine.is_empty() {
if prelude.contains("data-impeccable-variant") {
return Ok(None);
}
return Ok(Some(CssNode::Rule { prelude: prelude.to_string(), body: body.to_string() }));
}
let rewritten: Result<Vec<String>, String> = mine.iter().map(|sel| rewrite_selector(sel, anchor)).collect();
Ok(Some(CssNode::Rule { prelude: rewritten?.join(", "), body: body.to_string() }))
}
/// Nodes outside any `@scope` (the top level, or an `@media` / `@supports`
/// block at the top level): the accepted variant's `@scope` block is
/// flattened and rewritten, prefixed rules go through `global_rule`,
/// nested blocks recurse, and global at-rules (`@keyframes`, `@font-face`)
/// are kept.
fn global_nodes(nodes: &[CssNode], variant_num: &str, anchor: &str, out: &mut Vec<CssNode>, rules: &mut usize) -> Result<(), String> {
for node in nodes {
match node {
CssNode::At { name, prelude, children: Some(children), .. } if name == "scope" => {
let Some(caps) = SCOPE_PRELUDE_RE.captures(prelude) else {
return Err(format!("@scope block without a variant prelude: {}", prelude));
};
if &caps[1] == variant_num {
rewrite_nodes(children, anchor, &mut kept, &mut rules)?;
rewrite_nodes(children, anchor, out, rules)?;
}
}
CssNode::Rule { prelude, body } => {
// Astro's global-prefixed mode: `[data-impeccable-variant="N"] > .x`.
let mine: Vec<String> = split_selector_list(prelude)
.into_iter()
.filter(|sel| {
SCOPE_PRELUDE_RE
.captures(sel)
.map(|c| &c[1] == variant_num)
.unwrap_or(false)
})
.collect();
if mine.is_empty() {
if prelude.contains("data-impeccable-variant") {
continue;
}
kept.push(CssNode::Rule { prelude: prelude.clone(), body: body.clone() });
rules += 1;
continue;
if let Some(rule) = global_rule(prelude, body, variant_num, anchor)? {
out.push(rule);
*rules += 1;
}
let selectors: Result<Vec<String>, String> = mine.iter().map(|sel| rewrite_selector(sel, anchor)).collect();
kept.push(CssNode::Rule { prelude: selectors?.join(", "), body: body.clone() });
rules += 1;
}
CssNode::At { prelude, children: Some(children), name, .. } => {
// A media/supports block at the top level: keep only what is
// ours, rewritten.
CssNode::At { name, prelude, children: Some(children), .. } => {
let mut inner = Vec::new();
let mut inner_rules = 0usize;
for child in children {
if let CssNode::At { name: cn, prelude: cp, children: Some(cc), .. } = child {
if cn == "scope" {
if SCOPE_PRELUDE_RE.captures(cp).map(|c| &c[1] == variant_num).unwrap_or(false) {
rewrite_nodes(cc, anchor, &mut inner, &mut inner_rules)?;
}
continue;
}
}
if let CssNode::Rule { prelude: rp, .. } = child {
if rp.contains("data-impeccable-variant") {
continue;
}
}
inner.push(child.clone());
}
global_nodes(children, variant_num, anchor, &mut inner, &mut inner_rules)?;
if !inner.is_empty() {
kept.push(CssNode::At {
out.push(CssNode::At {
name: name.clone(),
prelude: prelude.clone(),
children: Some(inner),
body: None,
statement: false,
});
rules += inner_rules;
*rules += inner_rules;
}
}
other => kept.push(other.clone()),
other => out.push(other.clone()),
}
}
Ok(())
}
/// The accepted variant's rules out of the whole preview stylesheet: its
/// `@scope ([data-impeccable-variant="N"])` block rewritten and flattened,
/// its prefixed rules rewritten wherever they sit, global at-rules
/// (`@keyframes`, `@font-face`) kept, the other variants' rules dropped.
pub fn extract_variant_css(css: &str, variant_num: &str, anchor: &str) -> Result<(String, usize), String> {
let nodes = parse_stylesheet(css);
let mut kept: Vec<CssNode> = Vec::new();
let mut rules = 0usize;
global_nodes(&nodes, variant_num, anchor, &mut kept, &mut rules)?;
if rules == 0 {
return Err("the accepted variant declares no rules".to_string());
}
@@ -411,7 +444,12 @@ mod tests {
assert_eq!(rewrite_selector(":scope > .pricing-grid .pricing-card", a).unwrap(), ".pricing-grid .pricing-card");
assert_eq!(rewrite_selector(":scope .pricing-card", a).unwrap(), "div.pricing-grid .pricing-card");
assert_eq!(rewrite_selector(":scope", a).unwrap(), "div.pricing-grid");
assert_eq!(rewrite_selector(":scope:hover > .pricing-grid", a).unwrap(), "div.pricing-grid:hover > .pricing-grid");
assert_eq!(rewrite_selector(":scope:hover > .pricing-grid", a).unwrap(), ".pricing-grid:hover");
assert_eq!(rewrite_selector(":scope:focus-within > .pricing-grid .card", a).unwrap(), ".pricing-grid:focus-within .card");
assert_eq!(rewrite_selector(":scope[open] > .pricing-grid > .card", a).unwrap(), ".pricing-grid[open] > .card");
assert_eq!(rewrite_selector(":scope:hover .card", a).unwrap(), "div.pricing-grid:hover .card");
assert_eq!(rewrite_selector(":scope:not([hidden])", a).unwrap(), "div.pricing-grid:not([hidden])");
assert!(rewrite_selector(":scope:hover >", a).is_err());
assert_eq!(rewrite_selector("[data-impeccable-variant=\"2\"] > .x", a).unwrap(), ".x");
assert!(rewrite_selector(":scope + .x", a).is_err());
assert!(rewrite_selector(".a :scope", a).is_err());
@@ -447,6 +485,29 @@ mod tests {
assert!(!out.contains("data-impeccable") && !out.contains(":scope"), "{out}");
}
#[test]
fn prefixed_rules_under_a_top_level_media_block_are_rewritten_not_dropped() {
// Astro's global-prefixed mode, with a breakpoint.
let css = r#"
[data-impeccable-variant="1"] > .pricing-grid { gap: 8px; }
[data-impeccable-variant="2"] > .pricing-grid { gap: 32px; }
@media (max-width: 600px) {
[data-impeccable-variant="1"] > .pricing-grid { gap: 4px; }
[data-impeccable-variant="2"] > .pricing-grid { gap: 12px; }
[data-impeccable-variant="2"]:hover > .pricing-grid .card { border-color: #111; }
.site-wide { color: red; }
}
"#;
let (out, rules) = extract_variant_css(css, "2", "div.pricing-grid").unwrap();
assert_eq!(rules, 4, "{out}");
assert!(out.contains("@media (max-width: 600px)"), "{out}");
assert!(out.contains("gap: 12px"), "the variant's breakpoint survives: {out}");
assert!(out.contains(".pricing-grid:hover .card { border-color: #111; }"), "{out}");
assert!(out.contains(".site-wide { color: red; }"), "{out}");
assert!(!out.contains("8px") && !out.contains("4px"), "the other variant's rules are gone: {out}");
assert!(!out.contains("data-impeccable"), "{out}");
}
#[test]
fn knobs_and_plumbing_refuse_the_bake() {
let restored = vec!["<div className=\"pricing-grid\">".to_string(), "</div>".to_string()];
+18 -10
View File
@@ -73,6 +73,10 @@ pub struct AgentTargetPending {
pub payload: Value,
pub owner: Option<String>,
pub claimed_until: i64,
/// The overlay most recently granted the lease, kept when the lease
/// lapses or its page goes away: its Go may still land late, and it is
/// the only page besides the current holder allowed to open the session.
pub last_holder: Option<String>,
pub reports: Vec<AgentTargetReport>,
pub timer_gen: u64,
/// While every report says `no_match`, the roll call stays open until
@@ -821,6 +825,7 @@ impl ServerState {
payload: payload.clone(),
owner: None,
claimed_until: 0,
last_holder: None,
reports: Vec::new(),
timer_gen,
resolve_grace_until: None,
@@ -907,13 +912,14 @@ impl ServerState {
/// Why a generate event naming `envelope.targetId`, sent by
/// `envelope.clientId` under `session_id`, must not open a session:
/// the target is still pending but another page holds a live lease on
/// it (this page's lease lapsed while it was capturing); the request
/// was already answered, with a different session or with none (a
/// timeout or a failure verdict the CLI has already reported); or the
/// the target is still pending and this page is not its holder (another
/// page holds the lease, or held it last, or nobody claimed it); the
/// request was already answered, with a different session or with none
/// (a timeout or a failure verdict the CLI has already reported); or the
/// helper neither holds nor remembers the target (never issued here, or
/// long since evicted from the bounded record). None only when the
/// event is welcome: a pending target without a rival, or the
/// event is welcome: the holder's own Go (its lease may have lapsed, or
/// its page gone away, as long as no rescuer claimed since), or the
/// answering session's own event.
pub fn agent_target_refusal(
&self,
@@ -930,12 +936,13 @@ impl ServerState {
.iter()
.find(|(k, _)| k == target_id)
{
return match &pending.owner {
Some(owner) if owner != client_id && pending.claimed_until > now_i64() => {
Some(AgentTargetRefusal { session_id: None })
}
_ => None,
let is_holder = match &pending.owner {
Some(owner) => owner == client_id,
// Lease handed back or the page gone: only the page that
// held it last may still land its Go.
None => pending.last_holder.as_deref() == Some(client_id) && !client_id.is_empty(),
};
return if is_holder { None } else { Some(AgentTargetRefusal { session_id: None }) };
}
let Some((_, answered_by)) = self
.resolved_agent_targets
@@ -1104,6 +1111,7 @@ impl ServerState {
|| pending.claimed_until <= now;
if granted {
pending.owner = Some(client_id.to_string());
pending.last_holder = Some(client_id.to_string());
pending.claimed_until = now + lease_ms;
}
json!({ "ok": true, "granted": granted, "pending": 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` / `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"`, and `live-poll` renders that event's `_instructions` as the fast path (identity from the event's `element.computedStyles` / `cssCustomProperties` / `parentContext`, the action's three dimensions, no parameter knobs unless the prompt asks, one edit, reply done) instead of the interactive planning pointer and the action-reference read. 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 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 answering session's own event is welcome. |
| `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"`, and `live-poll` renders that event's `_instructions` as the fast path (identity from the event's `element.computedStyles` / `cssCustomProperties` / `parentContext`, the action's three dimensions, no parameter knobs unless the prompt asks, one edit, reply done) instead of the interactive planning pointer and the action-reference read. 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}`. |
| anything else | | 404 `Not found` |