Skip to content

fix(flow): verify complete typed input and stop on focus changes - #1831

Merged
abrichr merged 6 commits into
mainfrom
agent/1478
Nov 4, 2026
Merged

abrichr merged 6 commits into
mainfrom
agent/1478

Conversation

@abrichr

@abrichr abrichr commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Closes #1478

Typed values require an exact whitespace-squashed OCR match, including short values. When field bounds are unknown, readable-text verification covers the full row width so a crop cannot hide a prefix or suffix. Masked input keeps its local pixel crop, so an unrelated screen change cannot confirm delivery. Native and pixel-only runs halt without retrying unconfirmed delivery. The Windows agent checks the foreground window before and after each character or paste and reports uncertain delivery if it changes.

Runtime at 1b4d57d passes 428 privacy, pixel-scale, replay, Windows agent/backend and uncertain-delivery tests; full Flow Ruff, formatting, mypy (591 sources), and engine-generality checks pass. Negative controls reproduce the original acceptance/retry bugs, three cropped-value cases, and the masked-input screen-change regression.

Current head b96974a changes only the existing page-settle test stub to accept the new masked keyword. All four CI failures reproduce before it; all 22 page-settle tests pass after it. Every assertion is identical by AST audit. Full Flow Ruff/format pass at this head; runtime code is unchanged.

The synthetic backend reports the value it received. Pixel tests cover a record switch during readback (no-retry halt) and after verification (identity mismatch before saving). Main was integrated only to resolve two real append-only conflicts. The assertion audit retains all new main assertions and the prior no-retry strengthenings. No safety test is relaxed.

Live Windows desktop input was not tested. Update the guest agent to receive its foreground checks. CI and independent re-review are required for this head.

Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review: BLOCK

From: Grok. Reviewed head b73452f.

The typed-input checks themselves hold up. One change on the new no-retry halt path, though, makes the replayer fence a delivery that never happens.

Blocking

packages/flow/openadapt_flow/runtime/replayer.py:17723-17728: when can_retry is false, the halt path calls _delivery_authorization_refusal(...), and no input follows. That function is the last-point-before-delivery hook, and it has side effects:

  • it calls initial_guard.before_delivery() (replayer.py:12862);
  • it calls self._durable_continuation_guard.before_delivery() (replayer.py:12871);
  • on a consequential step it asks the approval gate before_save (replayer.py:12858);
  • when nothing refuses, it parks the guards in _active_delivery_acknowledgers (replayer.py:12887).

Because no delivery follows, those guards are never acknowledged. For a managed run, before_initial_delivery consumes the delivery fence and retains a pending remote permit (runtime/durable/authority.py:2638-2647). The durable record then says a second input crossing is in flight when none was sent, and the next attempt refuses with "a prior production delivery lacks an acknowledgment receipt" (authority.py:2634-2637). The run already halts, so this is not a safety bypass. But it writes a false delivery record into the durable authority and the approval gate.

Probe: I added a counting guard as _durable_continuation_guard and ran the scenario from test_opaque_typing_never_retries_an_unconfirmed_delivery (FakeBackend, pixels unchanged, exact_readback=False).

  • Actions: one click and one type, so 2 deliveries.
  • before_delivery ran 3 times; acknowledge_delivery ran 2 times.
  • One guard was left in _active_delivery_acknowledgers.
  • Error: "the target field showed no confirmed change; refusing an unleased refocus/retype retry after possibly stray input".

On main, this path either halts without the hook or retypes after it, so fences and acknowledgments match.

Suggested fix: in the not can_retry branch, don't call the delivery hook. If the "authorization no longer matches the current runtime inputs" message is still wanted (the updated test_typed_recovery_refuses_inputs_changed_during_verification, tests/test_replayer.py:1555), get it from a check with no fence or gate side effects, such as _fresh_actuation_authorization_refusal. Add a test asserting that before_delivery and acknowledge_delivery counts stay equal on this path.

Checked and fine

  • Exact OCR match: replayer.py:17613-17638. Truncated, prefixed, suffixed and short values are refused. Removing unchanged context lines works per line with multiplicity, so the remainder must be exactly the value.
  • if not changed: return False, False is now ahead of OCR (replayer.py:17609). A value already on screen can't verify a typing that changed nothing.
  • Retry is now limited to a web surface with a structural backend, an observable URL and a baseline readback. Opaque and native surfaces halt.
  • Windows agent: the foreground check runs before and after each ASCII character and around the paste (backends/win_agent/server.py:458-489). An unreadable foreground refuses before any typing. The client maps input_focus_changed to ActionDeliveryUncertain (backends/windows_backend.py:501).
  • Targeted tests on the head: tests/test_replayer.py and tests/test_win_agent_server.py with -k "typ or input or focus or foreground" gave 89 passed.

Non-blocking

  • identity.squash lowercases (runtime/identity.py:366-368), so "exact" OCR matching ignores case: "SMITH" verifies a typed "Smith". That was already true on main. Consider saying it in the docstring.
  • input_focus_unavailable is a 503, so the client reports it as uncertain delivery, even though the agent refused before typing anything. That fails closed, but a refusal would be more accurate.

@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Fixed the review blocker in 4c207c5. The no-retry halt now checks authority with _fresh_actuation_authorization_refusal, without acquiring a delivery fence or approval permit; governed refusal classification is preserved.

Both opaque-input cases reproduced 3 hook calls versus 2 deliveries/acks before the fix. They now assert exactly 2 hooks, 2 deliveries, 2 acknowledgments and no active acknowledgers. The existing late runtime-input mutation refusal still passes. Final validation: all 359 affected tests, Flow Ruff/format, and mypy (600 files) pass. The OCR docstring now states that matching lowercases text. Please re-review this head.

@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review: APPROVE

From: Grok. This re-reviews head 4c207c5, the fix for my BLOCK (6625560058).

The blocker is fixed:

  • On the no-retry halt path, packages/flow/openadapt_flow/runtime/replayer.py:17727-17739 now calls _fresh_actuation_authorization_refusal (defined at 12532-12581) instead of _delivery_authorization_refusal.
  • The new function only reads and compares, through these checks: the workflow and authorization snapshots, the profile, validate_workflow, the runtime inputs digest, and the program frame.
  • It doesn't call before_delivery on any guard. It also doesn't consult the approval gate or park acknowledgers.
  • So a halt no longer consumes a durable fence or a remote permit that no delivery follows.
  • The retry path, where a delivery does follow, still goes through _delivery_authorization_refusal as before.

The test covers it:

  • tests/test_replayer.py:4072-4099 adds a counting continuation guard to test_opaque_typing_never_retries_an_unconfirmed_delivery, for both exact_readback cases.
  • It asserts that before_delivery calls, acknowledgments and backend actions are all 2, and that _active_delivery_acknowledgers == ().
  • That is the 3-versus-2 imbalance my probe found.

A refused recheck still halts:

  • It sets safety_halt, plus failure_category governed_refusal under governed authorization and safety_halt otherwise.
  • A clean recheck returns the "could not be verified" halt.
  • The run ends either way, and nothing is delivered.

Tests: tests/test_replayer.py -k "typ or input or retry or deliver or ocr" on the head gives 66 passed.

Non-blocking:

  • The fresh recheck skips the managed-dispatch, production-qualification and campaign guards that the delivery check runs (12775 onward). On this path that changes only the halt message, because the step halts either way.
  • My two earlier notes still apply: the "exact" OCR match ignores case, which the docstring now says, and an input_focus_unavailable 503 is reported as uncertain delivery.

@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr
abrichr enabled auto-merge (squash) November 4, 2026 08:49
@abrichr
abrichr disabled auto-merge November 4, 2026 09:51
Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review: APPROVE

Re-review at the actual branch head 9e82eb0 (git ls-remote origin refs/heads/agent/1478). The PR API still shows 4c207c5. The only new commit is 9e82eb0 "read the full width of unknown text fields".

What I checked:

  • packages/flow/openadapt_flow/runtime/replayer.py:17469-17473: with no structural bounds, _field_region now returns (0, y, vw, h), a full-width band 240 px tall around the click. Before, it was a 640 px crop centred on the click, which could leave out a wrong prefix or suffix. This only makes the OCR check stricter. In exact_ocr_value (replayer.py:17611-17636), every changed OCR line in the band must add up to exactly the typed value. So text outside the old crop now causes a halt instead of being ignored. Structural regions (17461-17468) and the whole-frame case with no click (17469-17470) are unchanged.
  • tests/test_replayer.py:3914-3942: the three parametrised cases are a value in the field, a wrong prefix at x=20, and a wrong suffix at x=1400. The two wrong cases give landed is False, changed is True, and no backend actions, so there is no retype.
  • tests/test_pixel_scale_replay.py:92-130: the old identity-mismatch assertion is kept. A monkeypatch now runs the real _verify_typed_input first (and asserts it passes) before switching the record. The new test_record_switch_during_typed_readback_halts_without_retry asserts that a switch during readback halts with input_verified False, input_retried False, safety_halt True, and no save. Comparing git diff 4c207c5f7 9e82eb012 -- packages/flow/tests, the only removed assert line is that identity assertion, and it is re-added word for word.
  • tests/test_sanitized_artifact.py:132-146: the fake backend gains focused_text_value, so the type-only test goes through exact readback. This is a test double change, not a weaker assertion.

Ran locally at 9e82eb0: tests/test_replayer.py tests/test_pixel_scale_replay.py tests/test_sanitized_artifact.py, 218 passed. ruff check on the four changed files is clean.

Notes, non-blocking:

  • The masked (secret-only) path at replayer.py:17652-17657 now also works over the full-width band. An unrelated pixel change elsewhere in that band (a spinner, say) with no new readable text still counts as a landed secret. The old 640 px crop had the same risk, just over a narrower area. A test that covers a masked step with an unrelated change in the band would pin this down.
  • All three commits on the branch (b73452f, 4c207c5, 9e82eb0) carry a Signed-off-by trailer. Please drop it from the squash message.
  • There are no check runs on 9e82eb0 yet (commits/9e82eb012/check-runs total_count 0). This approval covers the code only; green CI on that sha is still needed.

@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
Signed-off-by: abrichr <richard.abrich@mldsai.com>
Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
@abrichr

abrichr commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review: APPROVE

From: Grok. This re-reviews head 1b4d57d. My last approval (6628601146) was at 9e82eb0. Two commits are new since then.

1d99fb0 (merge of main). git show --remerge-diff shows conflicts in only two files, and both resolutions keep both sides:

1b4d57d (masked input stays local).

  • packages/flow/openadapt_flow/runtime/replayer.py:17495-17499: _field_region(..., masked=True) now crops to MASKED_FIELD_REGION_WIDTH (640, line 405), centred on the click and clamped to the viewport. For unmasked input, w = vw, so x = min(max(0, px - vw//2), 0) = 0, which is the old (0, y, vw, h). Unmasked readback is unchanged.
  • replayer.py:17580: _typed_input_landed passes masked=allow_masked. The pixel-change gate (17615) and the readable-character check (17682-17685) both use that narrower region. A change outside it no longer confirms a masked entry. With no change inside it, the function returns (False, False) (17635-17636), so it halts or retries instead of accepting. That fails closed.
  • The structural-bounds branch (17487-17492) is untouched.

What I ran at 1b4d57d:

  • tests/test_replayer.py and tests/test_win_agent_server.py: 288 passed.
  • Control: I put 9e82eb0's replayer.py back. test_masked_input_needs_a_change_near_the_target[50-False] then fails: a change at x=50 confirmed a masked entry at x=900. The [900-True] case passes either way.
  • ruff check and ruff format --check are clean on the changed files.

Non-blocking. With no focusing click (field_point is None, e.g. Tab between fields), _field_region still returns None (17493-17494), so masked input is judged on the whole frame. That was already true at 9e82eb0, so this commit doesn't make it worse. The new comment at 17578-17579 ("an unrelated change elsewhere cannot confirm it") holds only when there is a click point. A follow-up could halt masked input with no field point and no structural region.

CI at 1b4d57d was still running when I checked: 5 success, 21 skipped, 1 neutral, 3 pending, none failed.

@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr
abrichr merged commit bd2af2d into main Nov 4, 2026
48 checks passed
@abrichr
abrichr deleted the agent/1478 branch November 4, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replay: typed-input checks accept partial OCR matches and retry after possibly stray input

1 participant