diff --git a/crates/tinybrowser-control/src/README.md b/crates/tinybrowser-control/src/README.md index 5249b99..e83a1fd 100644 --- a/crates/tinybrowser-control/src/README.md +++ b/crates/tinybrowser-control/src/README.md @@ -15,14 +15,15 @@ coordinate, script, or text value. The runner then applies deterministic policy: accept completion only when the independent Noul clears the threshold, and give one bounded reconsideration without `DONE` when it does not. A newly selected click still uses a ref from -the current snapshot and passes the irreversible-click gate. The runner also -stops offering `FILL` after a single caller-supplied input was successfully -entered on the current URL, so repeated fills cannot consume the unchanged-page -budget; navigation to a new URL makes that input available again. It -retries only browser errors the wire contract calls recoverable, stops after -repeatedly unchanged snapshots, and enforces a finite decision budget. The -browser engine still owns origin policy, current-ref validation, hit testing, -and input dispatch. +the current snapshot and passes the irreversible-click gate. A successful +single-input fill establishes a new post-fill snapshot, including any tree +change that shows the entered value. The runner stops offering `FILL` while +later observations match that snapshot and still contain the filled ref; a +subsequent page change makes the input available again. It also retries only +browser errors the wire contract calls recoverable, stops after repeatedly +unchanged snapshots, and enforces a finite decision budget. The browser engine +still owns origin policy, current-ref validation, hit testing, and input +dispatch. `test.rs` exercises policy and the loop with in-memory seams. The opt-in `tests/live_control.rs` test adds a real Chrome plus local page and mock System diff --git a/crates/tinybrowser-control/src/lib.rs b/crates/tinybrowser-control/src/lib.rs index 1b60339..2de877f 100644 --- a/crates/tinybrowser-control/src/lib.rs +++ b/crates/tinybrowser-control/src/lib.rs @@ -127,6 +127,33 @@ trait DecisionSource { ) -> Result; } +fn completed_single_input_on_page( + task: &TaskRequest, + snapshot: &Snapshot, + observation: Option<&(String, String, String)>, +) -> bool { + task.inputs.len() == 1 + && observation.is_some_and(|(url, title, tree)| { + url == &snapshot.url && title == &snapshot.title && tree == &snapshot.tree + }) +} + +fn fill_stayed_on_page( + decision: &Decision, + outcome: &StepOutcome, + before: &Snapshot, + after: &Snapshot, +) -> bool { + decision.operation == Operation::Fill + && matches!(outcome, StepOutcome::Acted) + && before.url == after.url + && before.title == after.title + && decision + .target + .as_ref() + .is_some_and(|target| after.refs.contains(target)) +} + impl DecisionSource for JevController { async fn decide( &self, @@ -223,11 +250,11 @@ impl JevController { let mut history = Vec::new(); let mut unchanged = 0_usize; let mut done_unconfirmed = false; - let mut filled_url = None; + let mut filled_observation: Option<(String, String, String)> = None; for step in 1..=self.limits.max_steps { let fill_already_entered = - task.inputs.len() == 1 && filled_url.as_deref() == Some(snapshot.url.as_str()); + completed_single_input_on_page(task, &snapshot, filled_observation.as_ref()); let decision = decisions .decide( task, @@ -281,8 +308,11 @@ impl JevController { let after = browser.snapshot(session, &snapshot_request).await?; let changed = policy::page_changed(&snapshot, &after); unchanged = policy::next_unchanged(unchanged, decision.operation, changed); - if decision.operation == Operation::Fill && matches!(&outcome, StepOutcome::Acted) { - filled_url = Some(snapshot.url.clone()); + if fill_stayed_on_page(&decision, &outcome, &snapshot, &after) { + filled_observation = + Some((after.url.clone(), after.title.clone(), after.tree.clone())); + } else if changed { + filled_observation = None; } history.push(StepRecord { step, diff --git a/crates/tinybrowser-control/src/test.rs b/crates/tinybrowser-control/src/test.rs index 5e76aef..0a52735 100644 --- a/crates/tinybrowser-control/src/test.rs +++ b/crates/tinybrowser-control/src/test.rs @@ -174,14 +174,14 @@ impl BrowserControl for FakeBrowser { #[derive(Debug)] struct FakeDecisions { decisions: Mutex>>, - offered: Mutex>, + context_flags: Mutex>, } impl FakeDecisions { fn new(decisions: impl IntoIterator) -> Self { Self { decisions: Mutex::new(decisions.into_iter().map(Ok).collect()), - offered: Mutex::new(Vec::new()), + context_flags: Mutex::new(Vec::new()), } } } @@ -195,9 +195,9 @@ impl DecisionSource for FakeDecisions { done_unconfirmed: bool, fill_already_entered: bool, ) -> impl std::future::Future> { - self.offered + self.context_flags .lock() - .expect("offered lock") + .expect("context flags lock") .push((done_unconfirmed, fill_already_entered)); std::future::ready( self.decisions @@ -394,8 +394,8 @@ async fn an_unconfirmed_done_reconsiders_the_visible_submit_button() { .tree .push_str("\ntextbox \"Query\" value=\"rust\" @e1"); let mut submitted = filled.clone(); - submitted.url = "https://example.com/results".to_owned(); - submitted.tree = "heading \"Results\" @e4".to_owned(); + // A title-only change on the same URL makes FILL available again. + submitted.title = "Results".to_owned(); let mut fill = decision(Operation::Fill, Some(element("e1", "textbox", "Query"))); fill.input_name = Some("query".to_owned()); @@ -419,9 +419,15 @@ async fn an_unconfirmed_done_reconsiders_the_visible_submit_button() { assert_eq!(result.steps[0].decision.operation, Operation::Fill); assert_eq!(result.steps[1].decision.operation, Operation::Click); assert_eq!(browser.actions.lock().expect("actions").len(), 2); + let contexts = decisions.context_flags.lock().expect("context flags lock"); + assert_eq!(contexts.len(), 4); + assert_eq!(contexts[0], (false, false), "initial Fill"); + assert_eq!(contexts[1], (false, true), "unconfirmed Done after Fill"); + assert_eq!(contexts[2], (true, true), "Click on the still-filled page"); assert_eq!( - *decisions.offered.lock().expect("offered lock"), - [(false, false), (false, true), (true, true), (false, false)] + contexts[3], + (false, false), + "confirmed Done after title change" ); } @@ -449,6 +455,115 @@ async fn repeated_unconfirmed_done_stops_within_the_decision_budget() { assert!(browser.actions.lock().expect("actions").is_empty()); } +#[tokio::test] +async fn filling_one_of_multiple_inputs_keeps_fill_available() { + let before = snapshot(); + let mut after = before.clone(); + after + .tree + .push_str("\ntextbox \"Query\" value=\"rust\" @e1"); + let mut fill = decision(Operation::Fill, Some(element("e1", "textbox", "Query"))); + fill.input_name = Some("query".to_owned()); + let decisions = FakeDecisions::new([fill, decision(Operation::Blocked, None)]); + let task = TaskRequest::new("fill query and site").with_inputs(BTreeMap::from([ + ("query".to_owned(), "rust".to_owned()), + ("site".to_owned(), "example.com".to_owned()), + ])); + let result = controller() + .run_with( + &FakeBrowser::new([before, after.clone()]), + &decisions, + &SessionId::new("session"), + &task, + ) + .await + .expect("task result"); + + assert_eq!(result.status, TaskStatus::Blocked); + assert_eq!( + *decisions.context_flags.lock().expect("context flags lock"), + [(false, false), (false, false)] + ); + let second_request = policy::build_request(&task, &after, &result.steps, false, false) + .expect("second field remains fillable"); + let Question::Choice(operations) = &second_request.questions["operation"] else { + panic!("operation must be a choice"); + }; + assert!(operations.criteria.contains_key("FILL")); + assert!(second_request.questions.contains_key("fill_input")); +} + +#[tokio::test] +async fn a_fill_that_navigates_keeps_the_input_available_on_the_new_page() { + let before = snapshot(); + let mut after = before.clone(); + after.url = "https://example.com/next-form".to_owned(); + after.title = "Next form".to_owned(); + let mut fill = decision(Operation::Fill, Some(element("e1", "textbox", "Query"))); + fill.input_name = Some("query".to_owned()); + let decisions = FakeDecisions::new([fill, decision(Operation::Blocked, None)]); + let task = TaskRequest::new("fill both forms") + .with_inputs(BTreeMap::from([("query".to_owned(), "rust".to_owned())])); + let result = controller() + .run_with( + &FakeBrowser::new([before, after.clone()]), + &decisions, + &SessionId::new("session"), + &task, + ) + .await + .expect("task result"); + + assert_eq!(result.status, TaskStatus::Blocked); + assert_eq!( + *decisions.context_flags.lock().expect("context flags lock"), + [(false, false), (false, false)] + ); + let next_request = policy::build_request(&task, &after, &result.steps, false, false) + .expect("input remains available"); + let Question::Choice(operations) = &next_request.questions["operation"] else { + panic!("operation must be a choice"); + }; + assert!(operations.criteria.contains_key("FILL")); +} + +#[tokio::test] +async fn a_same_url_form_replacement_keeps_the_input_available() { + let before = snapshot(); + let mut after = before.clone(); + after.tree = "textbox \"Next form\" @e1\nbutton \"Continue\" @e2".to_owned(); + after.refs = vec![ + element("e1", "textbox", "Next form"), + element("e2", "button", "Continue"), + ]; + let mut fill = decision(Operation::Fill, Some(element("e1", "textbox", "Query"))); + fill.input_name = Some("query".to_owned()); + let decisions = FakeDecisions::new([fill, decision(Operation::Blocked, None)]); + let task = TaskRequest::new("fill the next form") + .with_inputs(BTreeMap::from([("query".to_owned(), "rust".to_owned())])); + let result = controller() + .run_with( + &FakeBrowser::new([before, after.clone()]), + &decisions, + &SessionId::new("session"), + &task, + ) + .await + .expect("task result"); + + assert_eq!(result.status, TaskStatus::Blocked); + assert_eq!( + *decisions.context_flags.lock().expect("context flags lock"), + [(false, false), (false, false)] + ); + let next_request = policy::build_request(&task, &after, &result.steps, false, false) + .expect("input remains available on replacement form"); + let Question::Choice(operations) = &next_request.questions["operation"] else { + panic!("operation must be a choice"); + }; + assert!(operations.criteria.contains_key("FILL")); +} + #[tokio::test] async fn an_unconfirmed_form_goal_selects_the_current_submit_ref_and_preserves_approval() { let before = Snapshot { diff --git a/docs/specs/jev-browser-control.md b/docs/specs/jev-browser-control.md index 27efecc..8a28fa5 100644 --- a/docs/specs/jev-browser-control.md +++ b/docs/specs/jev-browser-control.md @@ -89,9 +89,10 @@ remain distinct and retain their sources. - One Jev request per decision, including an unconfirmed-completion retry and excluding provider retries internal to `tinyjevclient`. -- For a task with one caller-supplied input, a successful fill removes `FILL` - from the offered operations while the URL stays the same. A new URL permits - another fill with that input. +- For a task with one caller-supplied input, a successful fill establishes a + post-fill observation, even when entering the value changes the tree. `FILL` + stays unavailable while later observations match it and the filled ref still + names that field. A subsequent change or replacement permits another fill. - Only refs from the snapshot used for the decision may be acted on. - User-supplied input values are never used as Jev criterion identifiers. The model sees input names; the controller retains the values locally.