diff --git a/crates/puffer-cli/src/browser_action_verify.rs b/crates/puffer-cli/src/browser_action_verify.rs new file mode 100644 index 00000000..fe4be0ab --- /dev/null +++ b/crates/puffer-cli/src/browser_action_verify.rs @@ -0,0 +1,89 @@ +//! Shared post-condition verification helpers for browser connector actions. +//! +//! Change-type actions (send / delete / mark-read / RSVP) must re-navigate to +//! an authoritative view and assert the real outcome before reporting +//! success. Failures are built through [`verification_failure`] so the error +//! string carries the diagnostics (href/title/status/reason) returned by the +//! probe script or list poll -- the action error channel flattens errors to a +//! string, so diagnostics must live in the message itself. + +use serde_json::Value; + +/// Formats a post-condition verification failure, embedding the diagnostic +/// fields returned by the probe script or list poll into the error message. +pub(crate) fn verification_failure( + action: &str, + expectation: &str, + result: &Value, +) -> anyhow::Error { + let field = |key: &str| result.get(key).and_then(Value::as_str).unwrap_or("unknown"); + anyhow::anyhow!( + "Browser action `{action}` could not be verified: {expectation}; last URL `{}`, title `{}`, status `{}`, reason `{}`", + field("href"), + field("title"), + field("status"), + field("reason"), + ) +} + +/// Returns true when a Gmail list row refers to the given thread id. +/// +/// Action inputs may carry any of the id forms the inbox script extracts +/// (legacy hex, `thread-f:...` raw ids, or a `#`-prefixed hash fragment), so +/// the row's `threadId`/`legacyThreadId`/`gmailThreadId`/`id` fields are all +/// compared after normalization. +pub(crate) fn row_matches_thread(row: &Value, thread_id: &str) -> bool { + let expected = normalize_thread_id(thread_id); + if expected.is_empty() { + return false; + } + ["threadId", "legacyThreadId", "gmailThreadId", "id"] + .iter() + .filter_map(|key| row.get(*key).and_then(Value::as_str)) + .map(normalize_thread_id) + .any(|value| !value.is_empty() && value == expected) +} + +fn normalize_thread_id(value: &str) -> String { + value.trim().trim_start_matches('#').to_ascii_lowercase() +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn verification_failure_embeds_diagnostics_and_defaults() { + let result = json!({ + "href": "https://mail.google.com/mail/#sent", + "status": "loading", + "reason": "rows empty" + }); + let message = format!( + "{:#}", + verification_failure("send_email", "email not in Sent", &result) + ); + assert!(message.contains("send_email")); + assert!(message.contains("email not in Sent")); + assert!(message.contains("https://mail.google.com/mail/#sent")); + assert!(message.contains("status `loading`")); + assert!(message.contains("title `unknown`")); + } + + #[test] + fn row_matches_thread_normalizes_id_forms() { + let row = json!({ + "threadId": "18cabc123def4567", + "legacyThreadId": "18cabc123def4567", + "gmailThreadId": "#thread-f:1789012345678901234", + "id": "18cabc123def4567" + }); + assert!(row_matches_thread(&row, "18CABC123DEF4567")); + assert!(row_matches_thread(&row, "thread-f:1789012345678901234")); + assert!(row_matches_thread(&row, "#18cabc123def4567")); + assert!(!row_matches_thread(&row, "somethingelse")); + assert!(!row_matches_thread(&row, "")); + assert!(!row_matches_thread(&json!({}), "18cabc123def4567")); + } +} diff --git a/crates/puffer-cli/src/gcal_browser_actions.rs b/crates/puffer-cli/src/gcal_browser_actions.rs index 0ab7332a..86d21a7d 100644 --- a/crates/puffer-cli/src/gcal_browser_actions.rs +++ b/crates/puffer-cli/src/gcal_browser_actions.rs @@ -111,7 +111,7 @@ fn rsvp( let target = EventTarget::from_input(input)?; let handshake = super::ensure_browser_daemon(config, handshake)?; open_event(env, &account, handshake, &target)?; - let detail = wait_for_detail(env, &account, handshake)?; + wait_for_detail(env, &account, handshake)?; let result = evaluate_gcal_script(env, &account, handshake, &gcal_rsvp_script(response)) .with_context(|| format!("click Google Calendar RSVP `{response}`"))?; if !result.get("ok").and_then(Value::as_bool).unwrap_or(false) { @@ -123,13 +123,39 @@ fn rsvp( .unwrap_or("unknown reason") ); } - Ok(json!({ - "action": response, - "summary": format!("set Google Calendar RSVP to {response}"), - "account": account, - "clicked": result.get("label").cloned().unwrap_or(Value::Null), - "event": detail, - })) + // Post-condition: re-open the event (persistence, not the in-memory + // panel we just clicked) and require the RSVP control to show the + // selected state (#697/#698). + open_event(env, &account, handshake, &target)?; + let verified_detail = wait_for_detail(env, &account, handshake)?; + let deadline = Instant::now() + ACTION_TIMEOUT; + loop { + let verify = + evaluate_gcal_script(env, &account, handshake, &gcal_rsvp_verify_script(response)) + .with_context(|| format!("verify Google Calendar RSVP `{response}`"))?; + if verify.get("ok").and_then(Value::as_bool).unwrap_or(false) { + return Ok(json!({ + "action": response, + "summary": format!("set Google Calendar RSVP to {response}"), + "account": account, + "clicked": result.get("label").cloned().unwrap_or(Value::Null), + "event": verified_detail, + "verification": { + "matched": true, + "method": verify.get("method").cloned().unwrap_or(Value::Null), + "label": verify.get("label").cloned().unwrap_or(Value::Null), + }, + })); + } + if Instant::now() >= deadline { + return Err(crate::browser_action_verify::verification_failure( + response, + &format!("RSVP `{response}` was not reflected as selected in the event detail"), + &verify, + )); + } + std::thread::sleep(ACTION_INTERVAL); + } } fn open_event( @@ -227,13 +253,11 @@ fn wait_for_detail( return Ok(result); } if Instant::now() >= deadline { - bail!( - "Google Calendar event detail was not visible: {}", - result - .get("reason") - .and_then(Value::as_str) - .unwrap_or("detail panel not found") - ); + return Err(crate::browser_action_verify::verification_failure( + "get_detail", + "Google Calendar event detail was not visible", + &result, + )); } std::thread::sleep(ACTION_INTERVAL); } @@ -497,6 +521,59 @@ fn gcal_rsvp_script(response: &str) -> String { ) } +fn gcal_rsvp_verify_script(response: &str) -> String { + let labels = if response == "accept" { + json!(["yes", "accept", "going"]) + } else { + json!(["no", "decline", "deny", "not going"]) + } + .to_string(); + format!( + r#" +(() => {{ + const wanted = new Set({labels}); + const visible = (node) => {{ + if (!node) return false; + const rect = node.getBoundingClientRect(); + return rect.width > 0 && rect.height > 0; + }}; + const clean = (value) => String(value || "").trim().replace(/\s+/g, " "); + const labelFor = (node) => clean([ + node.getAttribute && node.getAttribute("aria-label"), + node.getAttribute && node.getAttribute("title"), + node.textContent + ].filter(Boolean).join(" ")).toLowerCase(); + const nodes = Array.from(document.querySelectorAll( + "button,[role='button'],[role='radio'],[aria-pressed],[aria-checked]" + )); + const candidates = []; + for (const node of nodes) {{ + if (!visible(node)) continue; + const label = labelFor(node); + if (![...wanted].some((item) => label === item || label.includes(item))) continue; + const selected = + node.getAttribute("aria-pressed") === "true" || + node.getAttribute("aria-checked") === "true" || + node.getAttribute("aria-selected") === "true" || + /(^|\s)(selected|checked)(\s|$)/i.test(node.className || ""); + candidates.push({{ label: label.slice(0, 60), selected }}); + if (selected) return {{ ok: true, label, method: "selected_state" }}; + }} + return {{ + ok: false, + reason: candidates.length + ? "RSVP button visible but not selected" + : "RSVP button not visible", + candidates: candidates.slice(0, 5), + href: location.href, + title: document.title || "", + bodyText: (document.body ? document.body.innerText || "" : "").slice(0, 200) + }}; +}})() +"# + ) +} + const GCAL_READY_SCRIPT: &str = r#" (() => { const href = location.href; @@ -530,7 +607,16 @@ const GCAL_DETAIL_SCRIPT: &str = r#" "[data-eid]" ].join(","))).filter(visible); const panel = panels.find((node) => /edit|delete|guest|calendar|going|yes|no|meet|location|description/i.test(node.innerText || "")); - if (!panel) return { ok: false, reason: "detail panel not found" }; + if (!panel) return { + ok: false, + reason: panels.length + ? "detail panel candidates were all rejected by the content gate" + : "detail panel not found", + candidateCount: panels.length, + href: location.href, + title: document.title || "", + bodyText: (document.body ? document.body.innerText || "" : "").slice(0, 200) + }; const titleNode = panel.querySelector("[role='heading'], h1, h2, [aria-level='1'], [aria-level='2']"); const bodyText = clean(panel.innerText || panel.textContent || ""); return { @@ -551,6 +637,23 @@ const GCAL_DETAIL_SCRIPT: &str = r#" mod tests { use super::*; + #[test] + fn detail_script_failure_carries_diagnostics() { + assert!(GCAL_DETAIL_SCRIPT.contains("candidateCount")); + assert!(GCAL_DETAIL_SCRIPT.contains("bodyText: (document.body")); + } + + #[test] + fn rsvp_verify_script_asserts_selected_state() { + let accept = gcal_rsvp_verify_script("accept"); + assert!(accept.contains("aria-pressed")); + assert!(accept.contains("aria-checked")); + assert!(accept.contains("going")); + let deny = gcal_rsvp_verify_script("deny"); + assert!(deny.contains("not going")); + assert!(deny.contains("RSVP button visible but not selected")); + } + #[test] fn canonicalizes_action_aliases() { assert_eq!(canonical_action("search_events"), "list_events"); diff --git a/crates/puffer-cli/src/gmail_browser_actions.rs b/crates/puffer-cli/src/gmail_browser_actions.rs index 9ab58b20..c32d9f61 100644 --- a/crates/puffer-cli/src/gmail_browser_actions.rs +++ b/crates/puffer-cli/src/gmail_browser_actions.rs @@ -6,16 +6,22 @@ mod gmail_browser_draft; use anyhow::{Context, Result}; use gmail_browser_draft::{ draft_rows_contain, gmail_reply_draft_script, gmail_reply_draft_verify_script, - gmail_save_draft_script, + gmail_save_draft_script, sent_rows_contain, }; use serde_json::{json, Value}; -use std::time::Instant; +use std::time::{Duration, Instant}; use super::{ ensure_browser_daemon, poll_account_at_url, safe_session_part, GmailBrowserConfig, - SubscriberEnv, BROWSER_HEIGHT, BROWSER_WIDTH, GMAIL_EVALUATE_INTERVAL, GMAIL_LOAD_TIMEOUT, + SubscriberEnv, BROWSER_HEIGHT, BROWSER_WIDTH, GMAIL_EVALUATE_INTERVAL, GMAIL_INBOX_SCRIPT, + GMAIL_LOAD_TIMEOUT, }; +/// Minimum time to let Gmail replace the transient pre-search inbox rows with +/// the actual search results before trusting a scrape (see +/// [`poll_gmail_search_settled`]). +const GMAIL_SEARCH_SETTLE: Duration = Duration::from_millis(2500); + /// Executes one Gmail-browser connector action through the managed Chrome profile. pub(super) fn handle_action( env: &SubscriberEnv, @@ -51,8 +57,35 @@ fn gmail_list_emails( let account = gmail_action_account(config, input)?; let url = gmail_collection_url(&account, input); let handshake_ref = ensure_browser_daemon(config, handshake)?; - let result = poll_account_at_url(env, &account, handshake_ref, &url)?; + let mut result = poll_account_at_url(env, &account, handshake_ref, &url)?; ensure_gmail_action_ready(&account, &result)?; + if let Some(query) = string_input(input, "query").filter(|value| !value.trim().is_empty()) { + // A search must clear two async hurdles before its rows can be trusted, + // and the first scrape loses both races (#582): + // 1. Route: on a cold tab Gmail boots to `#inbox` and only then + // applies the `#search` hash, so the first `href` is `#inbox`. + // 2. Rows: even once `#search` commits, Gmail swaps in the real + // results a beat later, so the rows are still the prior inbox. + // Reporting either transient state would be a fresh false-success, so + // wait for the `#search` route to commit AND the result rows to settle. + // We assert the `#search` route rather than the exact encoded fragment: + // Gmail re-normalizes the hash (percent-decoding operators like + // `newer_than:1d`), so an exact-fragment match would false-fail the very + // operator queries #582 enables. + // A *cold* tab boots Gmail to `#inbox` and drops the `#search` hash + // outright, so a direct open of the search URL never reaches search. + // The hash only commits as a *warm* client-side navigation, so if the + // first scrape is not yet on `#search`, boot Gmail on the inbox and + // re-open the search URL as a warm hash change before settling. + if !href_is_search(&result) { + let inbox_url = format!("{}#inbox", gmail_base_url(&account)); + poll_account_at_url(env, &account, handshake_ref, &inbox_url)?; + result = poll_account_at_url(env, &account, handshake_ref, &url)?; + ensure_gmail_action_not_auth_blocked(&account, &result)?; + } + result = poll_gmail_search_settled(env, &account, handshake_ref, action, &query, result)?; + ensure_gmail_action_ready(&account, &result)?; + } let limit = integer_input(input, "limit").unwrap_or(30).clamp(1, 100) as usize; let filters = GmailRowFilters::from_input(input); let rows = result @@ -84,15 +117,70 @@ fn gmail_mark_read( let thread_id = gmail_thread_id(input)?; let url = gmail_thread_url(&account, input, &thread_id); let handshake_ref = ensure_browser_daemon(config, handshake)?; + // Opening the thread IS Gmail's native mark-as-read mechanism; the defect + // was reporting success without checking the unread marker actually + // cleared (#591). open_gmail_url(env, &account, handshake_ref, &url)?; wait_gmail_thread_ready(env, &account, handshake_ref, &thread_id)?; - Ok(json!({ - "action": action, - "summary": format!("opened Gmail thread {thread_id} for {account} to mark it read"), - "account": account, - "thread_id": thread_id, - "url": url, - })) + let collection_url = gmail_collection_url(&account, input); + let outcome = + poll_gmail_list_until(env, &account, handshake_ref, &collection_url, |listing| { + mark_read_verification_state(listing_rows(listing).as_slice(), &thread_id) + == MarkReadVerification::Read + })?; + match outcome { + Ok(_) => Ok(json!({ + "action": action, + "summary": format!("marked Gmail thread {thread_id} read for {account}"), + "account": account, + "thread_id": thread_id, + "url": url, + "verification": { + "matched": true, + "method": "collection_unread_flag", + "collection_url": collection_url, + }, + })), + Err(listing) => { + let expectation = match mark_read_verification_state( + listing_rows(&listing).as_slice(), + &thread_id, + ) { + MarkReadVerification::Read => unreachable!("read state matched before timeout"), + MarkReadVerification::StillUnread => { + format!("thread `{thread_id}` still shows as unread in the list") + } + MarkReadVerification::Missing => format!( + "thread `{thread_id}` was not visible in the first page of the list, so the read state could not be verified" + ), + }; + Err(crate::browser_action_verify::verification_failure( + action, + &expectation, + &listing, + )) + } + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum MarkReadVerification { + Read, + StillUnread, + Missing, +} + +fn mark_read_verification_state(rows: &[Value], thread_id: &str) -> MarkReadVerification { + match rows + .iter() + .find(|row| crate::browser_action_verify::row_matches_thread(row, thread_id)) + { + Some(row) if row.get("unread").and_then(Value::as_bool).unwrap_or(false) => { + MarkReadVerification::StillUnread + } + Some(_) => MarkReadVerification::Read, + None => MarkReadVerification::Missing, + } } fn gmail_delete( @@ -118,13 +206,31 @@ fn gmail_delete( .unwrap_or("unknown") ); } - Ok(json!({ - "action": action, - "summary": format!("deleted Gmail thread {thread_id} for {account}"), - "account": account, - "thread_id": thread_id, - "url": url, - })) + // Post-condition: the thread must be visible in Trash. A positive Trash + // assertion beats "absent from inbox" -- the thread may simply be outside + // the first page window, which would pass vacuously (#588). + let trash_url = format!("{}#trash", gmail_base_url(&account)); + match poll_gmail_list_until(env, &account, handshake_ref, &trash_url, |listing| { + listing_contains_thread(listing, &thread_id) + })? { + Ok(_) => Ok(json!({ + "action": action, + "summary": format!("deleted Gmail thread {thread_id} for {account}"), + "account": account, + "thread_id": thread_id, + "url": url, + "verification": { + "matched": true, + "method": "trash_list", + "trash_url": trash_url, + }, + })), + Err(trash) => Err(crate::browser_action_verify::verification_failure( + action, + &format!("thread `{thread_id}` was not visible in Trash after clicking Delete"), + &trash, + )), + } } fn gmail_draft( @@ -311,7 +417,8 @@ fn gmail_send_email( anyhow::bail!("send_email requires `to`"); } let account = gmail_action_account(config, input)?; - let url = gmail_compose_url(&account, action, input); + let fields = GmailComposeFields::from_input(action, input); + let url = gmail_compose_url_for_fields(&account, &fields); let handshake_ref = ensure_browser_daemon(config, handshake)?; open_gmail_url(env, &account, handshake_ref, &url)?; wait_gmail_ready(env, &account, handshake_ref)?; @@ -325,12 +432,32 @@ fn gmail_send_email( .unwrap_or("unknown") ); } - Ok(json!({ - "action": action, - "summary": format!("sent Gmail email for {account}"), - "account": account, - "url": url, - })) + // Post-condition: the email must show up in the Sent list before we may + // report success. Clicking Send proves nothing (#578). + let sent_url = format!("{}#sent", gmail_base_url(&account)); + match poll_gmail_list_until(env, &account, handshake_ref, &sent_url, |listing| { + sent_rows_contain(&fields, listing) + })? { + Ok(_) => Ok(json!({ + "action": action, + "summary": format!("sent Gmail email for {account}"), + "account": account, + "url": url, + "verification": { + "matched": true, + "method": "sent_list", + "sent_url": sent_url, + }, + })), + Err(sent) => Err(crate::browser_action_verify::verification_failure( + action, + &format!( + "email `{}` to {:?} was not visible in Sent after clicking Send", + fields.subject, fields.to + ), + &sent, + )), + } } fn open_gmail_url( @@ -437,6 +564,49 @@ fn ensure_gmail_action_not_auth_blocked(account: &str, result: &Value) -> Result Ok(()) } +/// Re-navigates to `url` and polls its list rows until `matched` accepts the +/// response or [`GMAIL_LOAD_TIMEOUT`] elapses. Change-type actions +/// (send/delete/mark-read) share this loop to assert their post-condition +/// against an authoritative view. Returns the satisfying response, or the last +/// response as `Err` on timeout so the caller can attach action-specific +/// diagnostics through [`verification_failure`]. +fn poll_gmail_list_until( + env: &SubscriberEnv, + account: &str, + handshake: &crate::daemon::Handshake, + url: &str, + matched: impl Fn(&Value) -> bool, +) -> Result> { + let deadline = Instant::now() + GMAIL_LOAD_TIMEOUT; + loop { + std::thread::sleep(GMAIL_EVALUATE_INTERVAL); + let listing = poll_account_at_url(env, account, handshake, url)?; + ensure_gmail_action_not_auth_blocked(account, &listing)?; + if matched(&listing) { + return Ok(Ok(listing)); + } + if Instant::now() >= deadline { + return Ok(Err(listing)); + } + } +} + +/// Extracts the `rows` array from a Gmail list response, defaulting to empty. +fn listing_rows(listing: &Value) -> Vec { + listing + .get("rows") + .and_then(Value::as_array) + .cloned() + .unwrap_or_default() +} + +/// Returns true when any row in a Gmail list response refers to `thread_id`. +fn listing_contains_thread(listing: &Value, thread_id: &str) -> bool { + listing_rows(listing) + .iter() + .any(|row| crate::browser_action_verify::row_matches_thread(row, thread_id)) +} + fn gmail_action_account(config: &GmailBrowserConfig, input: &Value) -> Result { let requested = string_input(input, "account").or_else(|| string_input(input, "account_slug")); if let Some(account) = requested { @@ -550,11 +720,6 @@ fn gmail_url_targets_thread(url: &str, thread_id: &str) -> bool { || (last_segment.len() >= 12 && last_segment.chars().all(|c| c.is_ascii_hexdigit())) } -fn gmail_compose_url(account: &str, action: &str, input: &Value) -> String { - let fields = GmailComposeFields::from_input(action, input); - gmail_compose_url_for_fields(account, &fields) -} - fn gmail_compose_url_for_fields(account: &str, fields: &GmailComposeFields) -> String { let mut pairs = vec![ ("authuser".to_string(), account.to_string()), @@ -620,9 +785,122 @@ impl GmailComposeFields { } } +/// Re-scrapes the current Gmail list tab in place (no re-navigation) with the +/// shared inbox script. +fn rescrape_gmail_list( + env: &SubscriberEnv, + account: &str, + handshake: &crate::daemon::Handshake, +) -> Result { + let root_session = format!("gmail-browser-{}", safe_session_part(&env.topic)); + let value = crate::daemon_browser::send_daemon_request( + handshake, + "browser_agent", + json!({ + "action": "evaluate", + "sessionId": root_session, + "tabId": safe_session_part(account), + "width": BROWSER_WIDTH, + "height": BROWSER_HEIGHT, + "background": true, + "script": GMAIL_INBOX_SCRIPT, + }), + ) + .context("re-scrape Gmail search results")?; + Ok(value.get("value").cloned().unwrap_or(Value::Null)) +} + +/// Ordered thread-id signature of a listing's rows, used to detect when a +/// search's result set has stopped changing. +fn row_signature(result: &Value) -> String { + listing_rows(result) + .iter() + .filter_map(|row| { + ["threadId", "legacyThreadId", "gmailThreadId", "id"] + .iter() + .find_map(|key| row.get(*key).and_then(Value::as_str)) + }) + .collect::>() + .join(",") +} + +fn href_is_search(result: &Value) -> bool { + result + .get("href") + .and_then(Value::as_str) + .is_some_and(|href| href.contains("#search")) +} + +/// Waits for a Gmail search to commit its `#search` route AND for its result +/// rows to settle before the scrape can be trusted. +/// +/// `initial` is the first scrape, which loses both races: on a cold tab its +/// `href` is still `#inbox`, and even on a warm tab the rows are still the +/// prior inbox until Gmail swaps in the results. Re-scrapes in place until the +/// `href` is on `#search` AND the row signature is stable across two +/// consecutive reads AND at least [`GMAIL_SEARCH_SETTLE`] has elapsed since the +/// route committed. On [`GMAIL_LOAD_TIMEOUT`], returns the last scrape if +/// `#search` was reached, otherwise a [`verification_failure`] so a dropped +/// query is reported honestly rather than as stale inbox rows (#582). +fn poll_gmail_search_settled( + env: &SubscriberEnv, + account: &str, + handshake: &crate::daemon::Handshake, + action: &str, + query: &str, + initial: Value, +) -> Result { + let deadline = Instant::now() + GMAIL_LOAD_TIMEOUT; + let mut latest = initial; + let mut prev_sig = row_signature(&latest); + // Settle is measured from when `#search` first commits, not from entry, so + // the cold-tab `#inbox` boot phase does not eat the settle window. + let mut search_committed_at = href_is_search(&latest).then(Instant::now); + loop { + std::thread::sleep(GMAIL_EVALUATE_INTERVAL); + let next = rescrape_gmail_list(env, account, handshake)?; + ensure_gmail_action_not_auth_blocked(account, &next)?; + let on_search = href_is_search(&next); + let sig = row_signature(&next); + let stable = on_search && sig == prev_sig; + if on_search && search_committed_at.is_none() { + search_committed_at = Some(Instant::now()); + } + prev_sig = sig; + latest = next; + if let Some(committed) = search_committed_at { + if stable && committed.elapsed() >= GMAIL_SEARCH_SETTLE { + return Ok(latest); + } + } + if Instant::now() >= deadline { + if href_is_search(&latest) { + return Ok(latest); + } + return Err(crate::browser_action_verify::verification_failure( + action, + &format!("Gmail search view for `{query}` was not reached"), + &latest, + )); + } + } +} + fn gmail_base_url(account: &str) -> String { - let encoded = url::form_urlencoded::byte_serialize(account.as_bytes()).collect::(); - format!("https://mail.google.com/mail/?authuser={encoded}") + // Gmail must be addressed by signed-in-account *index* (`/u/N/`), not by the + // `?authuser=` query form. The query form triggers a full-page + // redirect that resolves the account and *drops the URL hash fragment* on + // the way, silently landing on the default `#inbox`. That broke every + // non-inbox navigation built on this base -- `#search/...` (agentenv/ + // monorepo#582), and the plan-07 post-condition views `#sent` (send), + // `#trash` (delete), and `#inbox/` (mark_read / reply) -- because + // the intended view was never reached. The managed profile hosts a single + // signed-in account at index 0 (the old `?authuser=` already fell back to + // it regardless of the requested address), so `/u/0/` selects the same + // mailbox while preserving the fragment. Verified live: `/u/0/#search/...` + // reaches the search view where `?authuser=#search/...` did not. + let _ = account; + "https://mail.google.com/mail/u/0/".to_string() } fn gmail_drafts_url(account: &str) -> String { @@ -729,10 +1007,7 @@ struct GmailRowFilters { impl GmailRowFilters { fn from_input(input: &Value) -> Self { - let mut keywords = keywords_input(input); - if let Some(query) = string_input(input, "query") { - keywords.push(query); - } + let keywords = keywords_input(input); Self { from: string_input(input, "from").map(|value| value.to_ascii_lowercase()), subject: string_input(input, "subject").map(|value| value.to_ascii_lowercase()), @@ -922,15 +1197,45 @@ mod tests { fn collection_url_supports_query_category_and_label() { assert_eq!( gmail_collection_url("me@example.com", &json!({"category": "promotions"})), - "https://mail.google.com/mail/?authuser=me%40example.com#category/promotions" + "https://mail.google.com/mail/u/0/#category/promotions" ); assert_eq!( - gmail_collection_url("me@example.com", &json!({"query": "from:alice has:attachment"})), - "https://mail.google.com/mail/?authuser=me%40example.com#search/from%3Aalice+has%3Aattachment" + gmail_collection_url( + "me@example.com", + &json!({"query": "from:alice has:attachment"}) + ), + "https://mail.google.com/mail/u/0/#search/from%3Aalice+has%3Aattachment" ); assert_eq!( gmail_collection_url("me@example.com", &json!({"label": "Clients/Acme"})), - "https://mail.google.com/mail/?authuser=me%40example.com#label/Clients%2FAcme" + "https://mail.google.com/mail/u/0/#label/Clients%2FAcme" + ); + } + + #[test] + fn base_url_uses_path_index_so_hash_fragment_survives_navigation() { + // Regression (agentenv/monorepo#582 + plan-07): the `?authuser=` + // query form triggered a redirect that dropped the URL hash fragment, + // so `#search` / `#sent` / `#trash` / `#inbox/` navigations + // silently fell back to `#inbox`. Verified live that the `/u/N/` path + // form preserves the fragment where the query form did not. + let base = gmail_base_url("me@example.com"); + assert!( + !base.contains("authuser"), + "base must not use the fragment-dropping authuser query form: {base}" + ); + assert!( + base.starts_with("https://mail.google.com/mail/u/"), + "base must be a /u/N/ path form: {base}" + ); + // The plan-07 post-condition views must carry their fragment intact. + assert_eq!( + format!("{base}#sent"), + "https://mail.google.com/mail/u/0/#sent" + ); + assert_eq!( + format!("{base}#trash"), + "https://mail.google.com/mail/u/0/#trash" ); } @@ -945,7 +1250,7 @@ mod tests { }), "19ef88112d77ab50", ), - "https://mail.google.com/mail/?authuser=me%40example.com#inbox/19ef88112d77ab50" + "https://mail.google.com/mail/u/0/#inbox/19ef88112d77ab50" ); } @@ -967,8 +1272,7 @@ mod tests { #[test] fn compose_url_includes_cc_bcc_subject_and_body() { - let url = gmail_compose_url( - "me@example.com", + let fields = GmailComposeFields::from_input( "draft_reply", &json!({ "to": ["alice@example.com"], @@ -978,6 +1282,7 @@ mod tests { "body": "Looks good", }), ); + let url = gmail_compose_url_for_fields("me@example.com", &fields); assert!(url.contains("to=alice%40example.com")); assert!(url.contains("cc=bob%40example.com")); assert!(url.contains("bcc=ops%40example.com")); @@ -1013,6 +1318,57 @@ mod tests { )); } + #[test] + fn sent_rows_require_subject_and_recipient_together() { + let fields = GmailComposeFields::from_input( + "send_email", + &json!({ "to": "bob@example.com", "subject": "Quarterly report", "body": "Numbers attached" }), + ); + // Subject AND recipient present in the row: match. + assert!(sent_rows_contain( + &fields, + &json!({"rows":[{"sender":"To: Bob","fromEmail":"bob@example.com","subject":"Quarterly report","snippet":"Numbers attached"}]}) + )); + // Same recipient but a different old email: draft_rows_contain's OR + // semantics would match this -- sent verification must not. + assert!(!sent_rows_contain( + &fields, + &json!({"rows":[{"sender":"To: Bob","fromEmail":"bob@example.com","subject":"Lunch","snippet":"See you"}]}) + )); + // Subject matches but recipient absent: no match. + assert!(!sent_rows_contain( + &fields, + &json!({"rows":[{"sender":"To: Carol","fromEmail":"carol@example.com","subject":"Quarterly report","snippet":""}]}) + )); + // Empty rows / fields without any signal never vacuously match. + assert!(!sent_rows_contain(&fields, &json!({"rows":[]}))); + let empty = GmailComposeFields::from_input("send_email", &json!({})); + assert!(!sent_rows_contain( + &empty, + &json!({"rows":[{"subject":"anything","snippet":"x"}]}) + )); + } + + #[test] + fn mark_read_verification_requires_matching_row_with_unread_cleared() { + let rows = vec![ + json!({"threadId":"thread-f:read","unread":false}), + json!({"threadId":"thread-f:unread","unread":true}), + ]; + assert_eq!( + mark_read_verification_state(&rows, "#thread-f:read"), + MarkReadVerification::Read + ); + assert_eq!( + mark_read_verification_state(&rows, "thread-f:unread"), + MarkReadVerification::StillUnread + ); + assert_eq!( + mark_read_verification_state(&rows, "thread-f:missing"), + MarkReadVerification::Missing + ); + } + #[test] fn row_filters_match_sender_subject_unread_and_keywords() { let row = json!({ @@ -1033,6 +1389,30 @@ mod tests { assert!(!filters.matches(&row)); } + #[test] + fn row_filters_do_not_swallow_query_operators() { + // #582: `query` is interpreted server-side via the #search URL; treating + // it as a literal client-side keyword made operator queries like + // `newer_than:1d` match zero rows. + let row = json!({ + "sender": "Alice", + "fromEmail": "alice@example.com", + "subject": "Launch plan", + "snippet": "Budget attached", + "unread": true + }); + let filters = GmailRowFilters::from_input(&json!({ "query": "newer_than:1d" })); + assert!(filters.matches(&row)); + // Explicit `keywords` keep their client-side filter contract. + let filters = GmailRowFilters::from_input(&json!({ + "query": "newer_than:1d", + "keywords": ["budget"] + })); + assert!(filters.matches(&row)); + let filters = GmailRowFilters::from_input(&json!({ "keywords": ["missing"] })); + assert!(!filters.matches(&row)); + } + #[test] fn action_account_ignores_connector_connection_slug() { let config = GmailBrowserConfig { diff --git a/crates/puffer-cli/src/gmail_browser_draft.rs b/crates/puffer-cli/src/gmail_browser_draft.rs index 84376a80..e87a0523 100644 --- a/crates/puffer-cli/src/gmail_browser_draft.rs +++ b/crates/puffer-cli/src/gmail_browser_draft.rs @@ -42,6 +42,47 @@ pub(super) fn draft_rows_contain(fields: &GmailComposeFields, result: &Value) -> }) } +/// Returns true when a Gmail Sent list response contains a row matching the +/// just-sent email. Unlike [`draft_rows_contain`] (OR semantics, good enough +/// for drafts), sent verification requires subject AND at least one +/// recipient to co-occur in the same row, so an unrelated older email to the +/// same recipient cannot vouch for this send. +pub(super) fn sent_rows_contain(fields: &GmailComposeFields, result: &Value) -> bool { + let rows = result + .get("rows") + .and_then(Value::as_array) + .cloned() + .unwrap_or_default(); + let expected_subject = fields.subject.trim().to_ascii_lowercase(); + let expected_recipients = fields + .to + .iter() + .chain(fields.cc.iter()) + .chain(fields.bcc.iter()) + .map(|value| value.trim().to_ascii_lowercase()) + .filter(|value| !value.is_empty()) + .collect::>(); + if expected_subject.is_empty() && expected_recipients.is_empty() { + return false; + } + rows.iter().any(|row| { + let haystack = [ + row.get("sender").and_then(Value::as_str).unwrap_or(""), + row.get("fromEmail").and_then(Value::as_str).unwrap_or(""), + row.get("subject").and_then(Value::as_str).unwrap_or(""), + row.get("snippet").and_then(Value::as_str).unwrap_or(""), + ] + .join(" ") + .to_ascii_lowercase(); + let subject_ok = expected_subject.is_empty() || haystack.contains(&expected_subject); + let recipients_ok = expected_recipients.is_empty() + || expected_recipients + .iter() + .any(|recipient| haystack.contains(recipient)); + subject_ok && recipients_ok + }) +} + /// Builds a Gmail compose autosave script for standalone draft windows. pub(super) fn gmail_save_draft_script(fields: &GmailComposeFields) -> String { draft_script(fields, false) diff --git a/crates/puffer-cli/src/main.rs b/crates/puffer-cli/src/main.rs index e7a2b0d0..6ace2ffa 100644 --- a/crates/puffer-cli/src/main.rs +++ b/crates/puffer-cli/src/main.rs @@ -6,6 +6,7 @@ mod basic_commands; mod benchmark_reflection; mod benchmark_run; mod browser; +mod browser_action_verify; mod browser_args; mod browser_output; mod browser_profiles;