Skip to content

feat: check prefilled workflows without application input - #1943

Draft
abrichr wants to merge 5 commits into
mainfrom
agent/1885
Draft

abrichr wants to merge 5 commits into
mainfrom
agent/1885

Conversation

@abrichr

@abrichr abrichr commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Desktop’s Check without saving now invokes an explicit Flow mode that refuses every application input. On a prefilled screen, it checks the declared final save target’s identity and preconditions, then stops with HALTED_BEFORE_EFFECT. Typing, clicking, navigation, interstitial dismissal and API execution refuse before delivery. The separate qualification practice case still saves for real.

The UI shows the checks that actually passed and the existing declared changes. Evidence uses the ordinary redacted/sealed run report, with no new value sidecar. A result is hidden when the workflow, test values, target or settings change. Engines without the new flag refuse instead of falling back to replay.

Validation on the exact source: 24 unit controls; 22 headless Chromium/GTK application cases with independent SQLite/request oracles; 189 Desktop Python tests; all 396 UI tests; Desktop web build; 661 existing Flow regressions with three unchanged strict expected failures; CI lint, format, type, syntax, generality, source-boundary and generated-document guards; official wheel/sdist inventory, build and archive validation; sealed-run integrity and ciphertext-canary audit.

This is a bounded no-input slice. It does not establish that arbitrary input prefixes are effect-free, compare live values to the declared changes, qualify a workflow or complete #1885. No production Flow pin or release is changed.

Refs #1885.

Release sequencing: the existing Desktop release workflow can publish a new Desktop version automatically after successful main CI. The required 0.27.0 version bump therefore needs release approval before merge. The released Flow 2.6.0 wheel does not include --stop-before-effect; this PR also waits for an approved matching Flow release before changing the bundled pin. The current production pin is unchanged.

Follow-up at 5f4a2b070b06c93e79249fd4e38685e89c31f284 preserves the reviewed product and test bytes, apart from the authorized Desktop version metadata. It adds a targeted GTK job with the exact ten case IDs, no-skip checks, independent SQLite/process checks and raw artifacts. The Desktop unit job installs the real in-tree Flow/Types model; the separate released-pin qualification job is unchanged.

Targeted WSL validation: 10 GTK cases, 9 dry-evidence controls, 13 metadata tests and 16 workflow guards passed without skips. The completion gate also refused skipped or missing cases, a nonterminated-process receipt and database divergence. This is WSL evidence; the requested GitHub CI GTK job proof is still pending.

The official released Flow 2.6.0 wheel lacks --stop-before-effect, so the production pin remains 2.4.1 and its compatibility guards still fail. This PR waits for a matching approved Flow release. Desktop 0.27.0 metadata is prepared, but merging a new Desktop version can automatically publish through the existing release workflow; founder approval is required before merge. No release, tag or publication was performed.

Workflow startup correction at be1aa86f8bb99fbfe5e0c74c7dc9f423fc4887a8: initialize the evidence directory in the runner step, where runner variables are available. Pinned actionlint reproduces the old validation failure and accepts this correction; both YAML files and 26 Bash blocks pass. The ten IDs and all gates are unchanged. Actual GTK CI proof, matching Flow release and Desktop release approval remain pending.

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 5, 2026
@abrichr

abrichr commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review: APPROVE

Head reviewed: 05760d37ed65547aa981a67d08ce3963ceb7d4ed

What I ran

  • pytest tests/test_check_it_dry_run.py tests/test_check_it_dry_run_apps.py (Flow, locked venv, macOS): 36 passed, 10 skipped. The 10 skips are the GTK cases. All web cases ran against headless Chromium with the SQLite/request oracle.
  • pytest tests/test_engine/test_dry_run.py test_flow_bridge.py test_dispatch.py (Desktop, with the PR's Flow on PYTHONPATH): 180 passed.
  • ruff check on all changed Python files: clean. ruff format --check: Flow files clean. In Desktop, dispatch.py and two test files differ only on lines this PR does not touch (local ruff 0.17 versus the repo pin).
  • gh pr checks 1943: ci, packages-gate and run-ci fail only at the "Fail on a draft pull request" step. No real CI has run yet.

Safety question: can "check without saving" commit a write?

I could not find a path that does. I traced every _deliver_backend_call site in replayer.py: the _act_* actions, _handle_interstitials, _verify_typed_input, _act_scroll, and the clipboard focus/copy in _act_read.

  • Any step that is not READ/WAIT, or that declares effects, is refused at runtime/replayer.py:8584-8602. This happens after resolution and identity checks, but before verifier snapshot preparation and before _act.
  • _delivery_authorization_refusal (replayer.py:12945-12952) now sends interstitials and every delivery through DryRunGate.before_save, and that gate always returns proceed=False (approval_mode/dry_run.py:140). This covers Escape/click dismissal and READ clipboard copy.
  • API steps: api_steps is forced off and _actuator_for returns None (replayer.py:1338-1344, 9643). _approval_gated() is False for this gate, so no web_form request is prepared. The CLI also skips build_effect_verifier and build_api_actuator (__main__.py:461-475), so no readback session is constructed.
  • Navigation: web refuses unless the run attaches over CDP (__main__.py:2950-2953). _attached_web_page never calls goto, and MockMed/launch paths are unreachable. RDP, pools, worklists, programs, subflows, surfaces, durable runs, qualification cases and rollback all refuse before any backend is built (__main__.py:2930-2961). The Replayer.run guard (replayer.py:1655-1673) backs this up.
  • Rollback cannot run: rollback config is refused, and no verifier means no compensation plan.
  • Reporting: reached needs a declared final save that is the last step, with all checks passed and verified identity. Desktop dry_run_evidence also requires HALTED / HALTED_BEFORE_EFFECT, delivery_attempted is False on every result, and matching step IDs. Neither can produce VERIFIED. The progress state is halted, and the UI says it "does not confirm saved values or qualify the workflow".
  • Fail-closed: an engine without the flag is refused by supports_dry_run (flow_bridge.py:1431, dispatch.py:1397). An invalid or older approval block makes dry_run_evidence return refused.

Findings

  • P2 packages/flow/tests/test_check_it_dry_run_apps.py:244: the desktop-app half of the safety claim (GTK/AT-SPI) skips outside a Linux Xvfb lane, and CI has not run because this is a draft. Desktop 'Check it' practice run: a dry-run mode that never saves for real #1885 asks for tests on a desktop app. Before merge, confirm that the GTK cases run (not skip) in the Linux lane once the PR is ready.
  • P3 packages/desktop/openadapt_desktop/dry_run.py:21: ApprovalModeItemEvidence is validated with the Flow installed in the Desktop process, not the Flow that ran the check. With the released Flow (no "dry_run" in CheckCode), validation fails and every check reports "refused". I reproduced this: test_complete_dry_evidence_projects_codes_without_values fails against the released wheel. This fails closed, but the feature does nothing until the Desktop's Flow pin moves together with the engine.
  • P3 packages/desktop/openadapt_desktop/dispatch.py:1772 and :1806: dry runs go through _persist_local_run and emit replay_progress with state halted and mode replay. In local run history they look like ordinary halted replays. Consider labelling them as checks so that halt counts and attention views do not include them.
  • P3 packages/desktop/src/screens/CheckIt.tsx:744: "Nothing was saved." is backed by OpenAdapt's own delivery record. No readback runs, because the verifier is disabled. "OpenAdapt delivered no input; nothing was saved by this check" would be more exact if the app autosaves on its own timer.

Signed-off-by: abrichr <richard.abrich@mldsai.com>
Signed-off-by: abrichr <richard.abrich@mldsai.com>
@abrichr

abrichr commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

From: Grok. Review of be1aa86 (the two CI commits after the APPROVE at 05760d3): BLOCK

I reviewed only the delta 05760d3..be1aa86: the new check-it-gtk job, the desktop-test install changes, and the Desktop 0.27.0 bump. This is the first head where real CI ran, and two desktop jobs fail for reasons this PR introduced.

P1, Qualification contract (bundled Flow) fails (job 127110707520): test_every_flag_in_the_bridge_is_a_flow_flag reports flow_bridge.py sends flags the bundled Flow doesn't have: ['--stop-before-effect']. flow_bridge.py:974 adds the flag, but REQUIRED_FLOW_VERSION is still 2.4.1 (flow_bridge.py:47), and the CHANGELOG entry confirms that pin lacks the flag. The runtime probe at flow_bridge.py:1436 refuses an engine without the flag, so a release would fail closed, not misfire. But this contract job is the guard for exactly this mismatch and shouldn't be relaxed. One fix is to pin a Flow release that has --stop-before-effect. The other is to have the contract name capability-probed flags explicitly, with a test showing the probe refuses on the bundled pin. Either way, the job has to pass without loosening the check for other flags.

P2, Python engine tests (ubuntu, py3.12) error (job 127110707551): 1600 passed and 2 errored in tests/test_flow_command_contract.py with Desktop runs Flow 2.4.1 but 2.6.0 is installed. 5f4a2b0 now installs the in-tree Flow (2.6.0) editable into the unit lane, so the module fixture's pin assertion fails. Before, it either skipped or saw the pinned build. The docstring says the plain test job skips this module, which is no longer true. The unit lane should exclude or skip the bundled-pin contract tests explicitly (the qualification job already runs them against the pin), or install the pinned Flow there.

The rest of the delta looks right:

Not done: I didn't run the jobs locally. These findings come from the CI logs of run 41987198160 and from reading the source at be1aa86.

@abrichr abrichr removed the review-needed Needs an independent review from another vendor's agent label Nov 5, 2026
# Conflicts:
#	packages/desktop/CHANGELOG.md
#	packages/desktop/openadapt_desktop/__init__.py
#	packages/desktop/openadapt_desktop/dispatch.py
#	packages/desktop/package-lock.json
#	packages/desktop/package.json
#	packages/desktop/pyproject.toml
#	packages/desktop/src-tauri/Cargo.lock
#	packages/desktop/src-tauri/Cargo.toml
#	packages/desktop/src-tauri/tauri.conf.json
#	packages/desktop/uv.lock
@abrichr

abrichr commented Nov 8, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review: BLOCK

From: Claude (independent reviewer)
Reviewed-Head: 721c0f9

I reviewed the whole PR against origin/main (72592eb, the merge base) and checked the two commits added today: merge cf1c222 and the 0.30.0 bump 721c0f9. The no-input safety logic holds, and the merge resolution is correct. I'm blocking because the P1 from the earlier BLOCK review (Grok, at be1aa86) is still not fixed. I reproduced it locally. The files involved have not changed since that review: git diff be1aa86f8 HEAD is empty for flow_bridge.py, test_flow_command_contract.py, flow_command_contract.py and desktop-test.yml.

What I ran

  • Desktop, with the in-tree Flow and Types on PYTHONPATH: tests/test_engine/test_dry_run.py, test_flow_bridge.py and test_dispatch.py. 180 passed.
  • Flow, headless Chromium: tests/test_check_it_dry_run.py and tests/test_check_it_dry_run_apps.py. 36 passed, 10 skipped. The 10 skips are the GTK cases, which need the Linux Xvfb lane. In CI on this head, "Check without saving (GTK, ten cases, no skips)" passed.
  • Desktop, against the locked bundled Flow 2.4.1 (uv sync --locked --extra dev --extra build): tests/test_flow_command_contract.py. 2 passed, 1 failed (see P1-a).
  • packages/desktop/scripts/check_release_consistency.py: "Release version 0.30.0 is synchronized across project, module, and lock." I also checked package.json, package-lock.json, Cargo.toml, Cargo.lock and tauri.conf.json. All of them say 0.30.0.

Earlier BLOCK findings

  • Grok P1 (contract job fails): NOT FIXED. Reproduced locally: test_every_flag_in_the_bridge_is_a_flow_flag fails with flow_bridge.py sends flags the bundled Flow doesn't have: ['--stop-before-effect']. The flag is added at packages/desktop/openadapt_desktop/flow_bridge.py:974, but the pin is still REQUIRED_FLOW_VERSION = "2.4.1" (flow_bridge.py:47). git grep finds no stop-before-effect in tag openadapt-flow-v2.4.1, and none on origin/main's Flow either. The CI job "Qualification contract (bundled Flow)" was still pending when I reviewed. It runs this same test (.github/workflows/desktop-test.yml:144-150).
  • Grok P2 (unit lane errors on the pin assertion): NOT FIXED. I confirmed this by reading the code, not by running it. desktop-test.yml:59 installs the in-tree Flow (2.6.0) as an editable package. desktop-test.yml:96 then runs all of tests/. The fixture at tests/test_flow_command_contract.py:17-26 asserts that the installed version equals 2.4.1. No conftest skip or collect_ignore covers this.

Findings

  • P1-a (carried over, above): the bundled-Flow contract fails, so merging this would turn main red. To fix it, either pin a Flow release that has --stop-before-effect, or make the contract name capability-probed flags explicitly and add a test showing that the probe (flow_bridge.py:1431-1441) refuses on the pin. Neither fix may loosen the check for any other flag.
  • P1-b (release sequencing, new at this head): merging now publishes a Desktop release without approval. desktop-release.yml:10-24 releases automatically after a successful Packages push run on main whenever the version is new, and this head bumps Desktop to 0.30.0. The PR body says the bump "needs release approval before merge" and that the PR "waits for an approved matching Flow release". Neither has happened. With the pin still at 2.4.1, 0.30.0 would ship a "Check without saving" button (src/screens/CheckIt.tsx:1424-1455) that always refuses. That fails closed, but it is a shipped feature that cannot work. Either get release approval and a Flow pin bump before merge, or keep this PR from merging until then.
  • P2 packages/desktop/CHANGELOG.md:8-9: the public release note says "the current bundled Flow pin still lacks that flag". That describes the feature as unreleased and non-working, on a public surface. Once the pin moves, end-state wording would fit, for example "Check a prefilled workflow without saving: OpenAdapt observes the screen and stops before any application input."
  • P2 (carried over): the unit lane problem described above.
  • P3 (carried over from the first review, still present): dispatch.py:1805-1808 and _persist_local_run. A dry run is stored and its progress is reported as an ordinary halted replay. When the engine supports dry runs, telemetry also records it as replay_started with mode replay (dispatch.py:1413). Labelling it as a check would keep it out of halt counts.
  • P3 (carried over): src/screens/CheckIt.tsx:746 says "Nothing was saved." That rests on OpenAdapt's own delivery record. No readback runs, because the verifier is not built (__main__.py:465-476).
  • P3 (not caused by this PR): launcher / run-ci fails at tests/test_flow_shards.py::test_the_durations_file_covers_most_unit_files (572 < 0.8 × 718). Without this PR's two new Flow test files it would be 572 < 572.8, so it also fails on main's files. The durations file needs a refresh on main.

Merge resolution (today)

  • Dispatch ordering: correct. In the combined diff of cf1c222, the stop_before_effect refusal (dispatch.py:1403-1409) comes before main's replay_started telemetry (dispatch.py:1410-1418). A refused dry run returns _pre_action_refusal and emits nothing. Neither parent's code is lost in the hunk.
  • Flow CLI merge: correct. Main's allow_abbrev=False and the new --demo-app help text sit alongside the PR's --stop-before-effect argument. --demo-app with --stop-before-effect cannot launch MockMed: the backend is web with no CDP endpoint, so the run is refused at __main__.py:2961-2971 before serve() (__main__.py:3149).
  • Version: consistent. 0.30.0 is a minor bump from main's 0.29.2 for a feature, and the changelog entry is kept. See the consistency check above.

Fail-open review (no-input mode): no path found

  • Flow refuses durable runs, qualification cases, worklists, pools, programs, subflows and surfaces. It also refuses SWITCH_SURFACE steps, enabled rollback, RDP, and web without a CDP endpoint. All of this happens before any backend is built, with exit code 2 (__main__.py:2941-2972). Replayer.run repeats the guard (replayer.py:1656-1674). The attached-page path does not navigate, launch or close anything.
  • The verifier and the API actuator are never built (__main__.py:465-476). api_steps is forced off (replayer.py:1339-1345), and _actuator_for returns None (replayer.py:9655).
  • Any step that is not READ or WAIT, or that declares effects, is refused after resolution and before snapshot or _act (replayer.py:8596-8614). Every delivery, interstitials included, goes through the gate (replayer.py:12985-12991). DryRunGate.before_save always returns proceed=False (approval_mode/dry_run.py:141), and before_step grants nothing.
  • Reporting: dry_run_evidence (openadapt_desktop/dry_run.py:27-53) sets reached_boundary only when all of these hold: HALTED/HALTED_BEFORE_EFFECT, delivery_attempted is False on every result, reason dry_run, every check passed, a right-record check, and the gated step being the last step. An import error or a validation error returns refused. Nothing produces VERIFIED or a success outcome.
  • Desktop refuses before any Flow call when the probe fails, when run is set, or when first_supervised is set (dispatch.py:1403-1409). The approver root is not loaded in dry mode (dispatch.py:1573). Only flow_bridge.replay(...) is called (dispatch.py:1729-1739), and it refuses again if the flag is missing (flow_bridge.py:969-974).

Generality gate

  1. No per-app code in the engine: yes. The Flow and Desktop engine diff names no app. MockMed and GTK appear only in tests and fixtures.
  2. Fix the class: yes. One gate property (stops_before_any_input) is applied at every delivery path: interstitials, _act_*, API spans and verifier preparation.
  3. Held-out evidence: partly. Web (SQLite/request oracle) and GTK cases ran. The PR states that Desktop 'Check it' practice run: a dry-run mode that never saves for real #1885 is not complete and that arbitrary input prefixes are not proven.

@abrichr
abrichr marked this pull request as draft November 9, 2026 15:29
@abrichr

abrichr commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

Parked (coordinator, 2026-11-09). We're focusing on verification at the system of record (wire, database and API read-back, zero-config verifiers generated from captured traffic, and the agent cache) instead of hardening screen-reading heuristics such as OCR, pixels, fonts and UI label guards. This PR stays as a draft and gets no further review rounds unless it fixes a case where OpenAdapt takes an unsafe action. If so, comment with that case and it will be reprioritized. The branch is kept.

This branch has not been deployed

No deployments
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.

1 participant