Skip to content

feat: arm a create-new form's save by the form the run typed into - #1581

Merged
abrichr merged 14 commits into
mainfrom
claude/create-form-identity-v2
Oct 29, 2026
Merged

abrichr merged 14 commits into
mainfrom
claude/create-form-identity-v2

Conversation

@abrichr

@abrichr abrichr commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

Note (2026-10-29): this PR merged at b2af951. The "WebSockets and workers while a save is held" section and the limits below describe the follow-up, #1644, which is not on main yet. As merged at b2af951, the guard covers pages opened through launch(). On other paths (the qualify session, CLI surfaces, popups), a send the page kept on an already-open socket can still get past a held save. #1644 closes that.

The create-form identity: a labelled save on a create-new form is armed by the form the run typed into, so qualification and clinical-write stop refusing it as unarmed. This squashes #1543 onto main and fixes both of its reviews.

Re-review round 2: fixed (ded2cc2)

  1. Query string. action_sha256 covers origin, path and the sorted query parameter names. Query values are compared like untouched fields, as salted digests under ?name.

  2. What is sent. At arm time the submit guard stores, in the page, what the checked button would send: the address with its query, the method, and every name/value pair except exempt tokens. Every submit while the guard is armed is compared with that. The guard cancels:

    • a submit from another form;
    • the right form without its button;
    • contents that changed after the check (store-copy, disabled-toggle, delayed mutate-then-requestSubmit(button));
    • a second submit.
  3. Late submits. The guard stays armed after the click. The replayer watches it for 2 s, then reads and disarms it at the start of the next step. Any cancelled submit is a safety_halt. setTimeout submits are cancelled.

  4. Token exemption. A hidden or query value is exempt only when its name looks like a token and its value looks like one: at least 16 characters, not all digits. A short value under a token-like name (encounter_token=7) and a value a script sets when typing starts (pid) are both compared.

    One change from the review: the review also asked that an exempt value must have rotated during the recording. Trac's __FORM_TOKEN is per-session, so it doesn't rotate within a recording but differs on every new page load. Requiring rotation would false-halt nearly every CSRF-protected form. The name-and-shape rule closes both review rows without that.

  5. Minor fixes. File inputs count in the fingerprint, as the number of files. LIMITS.md now notes that low-entropy digests can be enumerated when the salt is stored, and that recordings hold untouched values in plain text.

Evidence

  • tests/test_create_form_identity.py has 24 real-Chromium cases. New ones:
    • the query swap;
    • store-copy, disabled-toggle, delayed other-form and delayed mutate are all cancelled with nothing submitted;
    • a rotating draft_rev, a script-set pid and encounter_token=7→9 are compared and halt;
    • a 24-character per-session __FORM_TOKEN still verifies.
  • The reviewer's unmodified test_probe_1543_r2.py, run against ded2cc2: every bold row now halts with nothing wrong submitted. Two of its control rows halt as well, by design:
    • store7 → store7: a click handler that fills a field as the form is sent changes what is sent after the check. That's a false halt in the safe direction, and LIMITS.md documents it.
    • csrf aaa → bbb: a three-character value can't be told apart from a record id. Real tokens are long and verify.
  • Local regression with one worker: the submit-binding, interactive-recorder, recorder, locator, policy, identity, qualification (without the gate campaign), replayer, compiler and fixup suites passed 1632, with 0 failures. mypy, ruff and gitleaks are clean.

Still open

  • A combobox that re-renders its field on selection (Medplum's gender autocomplete) false-halts, as the keyed remount does. It needs a replay on the real app.

What this branch has (as of #1543 81d00f5)

The problem

A form that creates a record (Trac's New Ticket, NetBox's Add Device) shows no existing record next to its save button. The identity band around Create ticket holds only the button's own label, so the save compiled unarmed ("the only readable row text is the target's own label"). Qualification then refused it ("save step … has no armed identity check").

The documented remedy, record --identifier FIELD, doesn't work here. I recorded a Trac-shaped form with --identifier field_summary, and the save was armed by a pixel crop of the summary field. Replay with a new summary then halted on an identity mismatch, because the crop pictures the demo's own value.

The fix

The record a create-form save commits is the one the run typed into the form. So the demonstrated form, all of it, is the identity.

  • Recording. For a labelled submit button of the form that owns every field typed since the last save, the submit-binding observer (submit_binding.py) records three things:

    • the form's relation to those fields;
    • a fingerprint: the submission's origin and path, its method, and the sorted name/type list of every control it would send, hidden ones included;
    • the values of the fields the run didn't type into.
  • Compile. The step gets Anchor.form_identity (FormIdentity), which holds only:

    • the filled-field count and form_submit;
    • SHA-256 digests of the address and the control list, plus the method;
    • salted digests of the untouched values (the identity-template scheme).
  • Hidden tokens are never compared, and their values are never kept. Two kinds are exempt: ones named like a token (csrf, token, nonce, …), whose value changes on every page load (Trac's __FORM_TOKEN is one), and hidden values that changed while the demonstration was typed. Password values never leave the page.

  • Replay. The new top identity tier (mode form) checks, in the DOM, right before the click, that:

    • the button is still that form's submit button;
    • the form owns exactly the fields this run typed into, each holding exactly what was typed;
    • the address, method and control list match the demonstration;
    • every untouched field holds its demonstrated value.

    Anything else is a mismatch and halts. A backend without form_identity_at leaves identity unreadable, which halts.

  • During the click. A one-shot capture-phase submit guard cancels any submit that is not that form from that button (for example a click handler's requestSubmit() on another form), and the step halts.

  • Signal quorum. A signal-quorum identity policy and a form identity must both hold.

  • Policy and reports. is_identity_armed, the replayer's identity gates, the armed-step count and the fixup pass all accept form_identity.

Tests (real headless Chromium)

tests/test_create_form_identity.py has 17 cases. Each records a Trac-shaped New Ticket form with the interactive recorder, compiles it with the real compiler, and replays it with the real Replayer.

Verifies and creates the right ticket:

  • new values;
  • a button outside the form with form=;
  • a matching hidden parent or select;
  • a per-session CSRF token that differs on the new page load;
  • a hidden value that rotated during the demonstration.

Halts before the click, nothing submitted:

  • the six holes from the review:
    • an edit form that looks like the create form (id=42, /ticket/42);
    • an address and method changed after typing;
    • a hidden id added while typing;
    • a hidden parent on another record;
    • a select left on another record (both reported as "the form's 'pid' field … holds a different value");
    • a click handler that submits another form (cancelled by the guard);
  • a field rewritten after typing;
  • a button of another form;
  • a portal button against a form= recording;
  • reformatting controlled inputs (as you type: stopped by the typed-value read-back; on blur: stopped by the form check);
  • a keyed React remount, pinned as a known false halt.

The reviewer's unmodified probe (test_probe_1543.py), run against this commit: all six holes halt with nothing submitted, and the controls (plain, parent_ok, select_ok, search_present) verify and submit.

Regression, run locally with one worker: the submit-binding, interactive-recorder, recorder, id-less locator, create-form, policy, identity, qualification (without the long gate campaign), replayer, compiler and fixup suites. 1646 passed, 11 skipped, 0 failed. ruff and mypy are clean.

Connections the wire check can't route

The wire check holds the save's request at the network. WebSockets, workers, WebTransport and WebRTC don't go through context.route, so a guard in the page holds them instead.

  • Cause of the CI failure. CI runs Playwright 1.61.0. In that version route_web_socket only routes sockets of documents loaded after it is registered. The backend registered it at the first save, while the form's document was already open, so the socket connected. Playwright 1.64 also patches open documents, which is why the test passed locally.
  • The page-side guard. prepare_ws_guard(context) installs an init script once per browser context. It runs before any page script in every document, and wraps WebSocket, WebTransport, RTCPeerConnection, Worker and SharedWorker. While a save is held, constructing one throws SecurityError, so nothing is created. A message sent on a WebSocket or RTCDataChannel that is already open is dropped. Each is reported, and the step halts. No prototype.constructor on a wrapped chain leads back to the unwrapped constructor. The guard decides at construction time in the page, so no outcome depends on when Python services the browser.
  • When the hold starts and ends. The hold is set in every open frame before the click, and documents loaded during the hold start held. It is released only after the outcome has been read.
  • Where the guard is installed. PlaywrightBackend.launch and every CLI, runner, qualification, benchmark and emitted-tool path that opens a page call prepare_ws_guard before the first goto. A popup shares its opener's context, so it is guarded too.
  • No WebSocket routing. WebSockets are no longer routed through Python at all. Playwright can't remove a WebSocket route, so one would stall every socket until Python serviced it, for the rest of the context. With nothing held, sockets connect directly.
  • Fails closed. A save is refused before the click when:
    • a document on the page loaded before the guard: "the page loaded before the WebSocket guard was installed, so the save can't be held". Its scripts may hold the browser's own constructors. Reloading the page after the backend exists clears this.
    • the guard can't be kept in a frame (for example WebSocket made non-configurable).
    • a dedicated worker runs, because a worker's WebSockets skip the guard.

Tests in tests/test_create_form_wire.py run each way the reviews found to reach the wire while held. With the guard installed from the first load, none of them sends anything:

  • prototype.constructor and the prototype chain;
  • new, blob and data iframes, and an about:blank window;
  • a Blob worker and a kept Worker;
  • a kept WebSocket, and a kept send on a socket opened before the backend existed;
  • sendBeacon and fetch with keepalive.

The tests also cover:

  • a page loaded before the guard is refused, and can hold once reloaded;
  • a page with a running worker is refused;
  • sockets connect within 3 s while Python is busy and nothing is held;
  • repeated holds on a reloading frame;
  • sockets reconnect after release, in old, popup and new documents.

They pass on Playwright 1.61.0 and 1.64.1.

Limits

  • A script that submits with form.submit() or fetch sends no submit event, so the guard doesn't see it. LIMITS.md says so.
  • A field that reformats the typed value halts, by design. So does a field replaced by a keyed remount.
  • A program that should create under a different parent on each run halts until that parent is checked another way.
  • Not held: the streams of a WebTransport session opened before the hold, and a shared worker that started before it. Neither can be seen from the page or the driver.
  • A page that runs a dedicated worker can't hold a create-form save. Apps that keep one running, such as pdf.js viewers, halt at the save. That's a known cost of failing closed.
  • A page that wasn't guarded from its first load can't hold a save. That includes any integration that hands the backend a page it navigated without calling prepare_ws_guard.
  • The guard is JavaScript in the page. It covers what page scripts can reach after it runs, which from the first load is everything they can reach.
  • This covers web forms only, and it doesn't prove the app saved the record correctly; only a record check does.

…rked)

Squashed from #1543 onto main. Parked under the feature pause; the
re-review's open findings are in the PR description.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, token scope

- The form's address includes its query parameter names; their values are
  compared like untouched fields (salted digests under "?name").
- The submit guard compares every submit with what the checked button
  would have sent (address with query, method, every name/value pair but
  exempt tokens): a click handler that copies a parent id into an empty
  field, enables a disabled id, or adds one before requestSubmit(button) is
  cancelled, and a second submit too.
- The guard stays armed after the click: the replayer watches it for 2 s,
  then reads and disarms it at the start of the next step, halting on any
  cancelled submit. setTimeout submits are cancelled.
- A hidden or query value is exempt only when named like a token and shaped
  like one (16+ characters, not all digits). A short value under a
  token-like name, or a value a script set while the form was filled in, is
  compared. File inputs count in the fingerprint.

Tests: the re-review's rows halt before the click with nothing submitted;
a per-session token still verifies. LIMITS.md: salt enumeration of
low-entropy digests, and the false halt of a page whose click handler
fills a field at submit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr abrichr changed the title feat: arm a create-new form's save by the form the run typed into (parked) feat: arm a create-new form's save by the form the run typed into Oct 28, 2026
@abrichr
abrichr marked this pull request as ready for review October 28, 2026 18:40
@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Re-review round 2 is addressed in ded2cc2; details are in the description. I ran your unmodified test_probe_1543_r2.py against it: every bold row halts with nothing wrong submitted.

Two of your control rows now halt by design:

  • store7 → store7: the click handler changes what is sent after the check.
  • csrf aaa → bbb: the exemption needs a token-shaped value, at least 16 characters and not all digits, to tell a token from encounter_token=7.

I also departed from point 4 on one thing: rotation during the recording isn't required. Requiring it would false-halt Trac's per-session __FORM_TOKEN. Ready for your re-check; auto-merge is off.

@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

AI review (coordinator): round 3 on ded2cc2

Verdict: one blocking hole remains. Not merged.

Earlier probes: all fixed

I re-ran both of the earlier reviewer's probes, unmodified, against ded2cc2 with real headless Chromium and one worker.

  • test_probe_1543.py: all 14 rows behave as the description says. The six holes halt with nothing submitted, and the controls verify and submit.
  • test_probe_1543_r2.py: every hole halts with nothing wrong submitted. This covers the query swap, a disabled id, the setTimeout submits, a form= hidden field outside the form, encounter_token=7→9, csrf injection, a file input, and a script-set pid.
  • submit_then_other halts after the correct submit and cancels the second one.
  • store7 → store7 and csrf aaa → bbb now halt, as you said. I accept both as safe-direction false halts.

New hole: the guard compares before the page's own submit listeners run

The guard is a capture listener on window, so it runs first. Page code that runs after it can still change what is sent, and the run reports verified / success. Probe file test_probe_1581_r3.py uses the r2 harness. I record on R, replay on V, and log what the form really sent:

rec → replay page change result sent
sub7 → sub9 form.addEventListener('submit', () => pid.value = currentPid, true) verified, success pid=9 (demo was 7)
plain → bubble42 document.addEventListener('submit', e => append hidden id=42, true) verified, success id=42, an edit-shaped submit
plain → fd9 form.addEventListener('formdata', e => e.formData.set('id','42')) verified, success id=42
  • sub9 is the store9 case moved from the button's click handler to the form's submit listener. Copying "current patient" state into a hidden field on submit is the more common SPA pattern.
  • fd9 passes for another reason. The guard's canon() builds new FormData(form, control), which fires formdata both at arm time and at submit time, so the injected pair is already in expected. verify_form_identity reads DOM controls, so it never sees the pair.

Possible fixes, any of which closes these rows:

  • Compare the entry list actually built for the submission, after page listeners have run. One way: listen for formdata on the form and compare event.formData. Another: cancel every submit in the guard and resubmit from the guard after a final canon() in a later task.
  • At arm time, cross-check that canon()'s pairs equal the DOM controls' names. That catches formdata injection before the click.

Add sub7→sub9, bubble42 and fd9 as regression cases.

Smaller points (not blocking)

  • Token rule. Name plus shape still exempts a record-shaped value under a token-like name. With encounter_token=enc-3f2a9c1e-0001 → …0002, the run verified and sent the other encounter. A 16-character, non-numeric UUID-style id is common. LIMITS.md should say that a token-named hidden field is never compared, whatever it holds.
  • Late refusals dropped. form_identity_at starts with cancel_form_submit_guard() and discards whatever it returns. If a guarded save follows another guarded save, a late refusal from the first is dropped without a halt. That submit was still cancelled. When the save is the last step, the guard stays armed with nothing reading it, so late3s reported success and its submit was cancelled silently. Treat a non-None result there as a halt.

No blocking finding outside the guard ordering. Size is under 1000 non-test lines, but I did not enable auto-merge because of the hole above.

🤖 Generated with Claude Code

…ked form

Round 3 of #1581's review: page listeners that run after every in-page
check (a form submit listener, a document capture listener, a formdata
listener) could still change what a save carries. So the Playwright
backend now routes every write request, and a GET navigation to the form's
address, through a wire guard from the identity check until the next step.
A request continues only when its method, address, query and fields
(URL-encoded, multipart or flat JSON) equal what the checked button would
send, and only once; anything else, including a shape it can't parse, is
aborted before it leaves the browser and halts the run.

The expected fields are built from the form's controls, never from
new FormData(), which would run the page's formdata listeners. A cancelled
submit found by the next form check, or at the end of the run, halts.
A record-shaped value (UUID-like hex groups) under a token-like name is
compared unless it changed while the demonstration was typed (the rotation
check is back for this).

Tests (test_create_form_wire.py): a server records what arrives; native
POST, fetch-JSON and the review's submit-listener, document-capture and
formdata cases. Both test files assert each save recorded armed, so a slow
recording can't pass a case for the wrong reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Round 3 is addressed in 9598072, by checking what is actually sent.

Wire guard. From the identity check until the next step, a Playwright route holds every write request the page sends, and any GET navigation to the form's address. A request leaves only when its method, address, query and fields equal what the checked button would send, and only once. URL-encoded, multipart and flat JSON bodies are compared; any other shape is held. Everything else is aborted before it leaves the browser and halts the run. The expected fields come from the form's controls, not new FormData(), so a formdata listener can't put its own field into the expected set.

New tests. tests/test_create_form_wire.py uses a server that records what actually arrives:

  • Get through, exactly once: a native POST, and a fetch-JSON save.
  • Never reach the server: sub7→sub9 (form submit listener), bubble42 (document capture), fd42 (formdata injection) and fetch→fetch42.

Your r3 probe, unmodified. uuidA→uuidB halts. The other rows still report success because those pages preventDefault and only log FormData in the page, so nothing is ever sent. In a real app, that handler would submit or fetch, and the wire tests show those requests are held.

Smaller points:

  • A late refusal found by the next form check, or at the end of the run, halts.
  • A record-shaped value under a token-like name is compared unless it rotated during the recording.
  • Remaining limit (in LIMITS.md): a submit a script delays past the end of the run is still cancelled, but the finished run doesn't report it.

Regression: 1663 passed with one worker; the one failure was a slow recording of the query case under load, which passes alone. Both test files now assert that each save recorded armed.

Checks inside a page run alongside the page's own scripts, so the guard
that compared submits in the page is gone. A checked save is now held at
the network by openadapt_flow.wire_check: a Playwright route lets the save
through only when its method, address, query and fields equal what the
checked form would send, once, and aborts anything else before it is
sent. The DOM checks before the click stay as pre-checks. The same module
serves taught browser writes (declared writes, pinned shapes, sign-in
values kept out of addresses in every encoding).

The create-form tests now submit for real to a server that records what
arrives, so each case asserts what left the browser.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Anchor.form_identity serialised as null into the sealed content and
changed the digest of every bundle sealed before it (the program-sections,
registry and loop showcase bundles failed to load in CI). Add the reviewed
v2 omission for an absent form identity, and the historical-seal check
for it. A recorded form identity stays sealed content.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abrichr added a commit that referenced this pull request Oct 28, 2026
From the #1589 review:
- A taught write's requests are pinned (method, path and the digest of its
  field names). Any other write is aborted before it is sent, while
  teaching and on every call, and the call ends in needs_review.
- Sign-in values never travel in an address and never reach the agent in
  any encoding (raw, percent, form, JSON, HTML, base64). They go only into
  a password field or a field of a POST sign-in form.
- Verified receipts are sealed with this computer's Seal key and say the
  values were declared by the agent and whether an operator's
  authorization was bound.

browser_teach now uses openadapt_flow.wire_check (from #1581) instead of
its own copy of the route guard, masking and body shapes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Round 4: one wire check, page-level guard removed.

  • Wire check (openadapt_flow/wire_check.py, shared with feat: verified writes for any agent, and taught browser writes with receipts #1589). After the form passes its DOM pre-check, a Playwright route holds every write request, plus a GET navigation to the form's address. A request leaves only when its method, address, query and every field (except exempt tokens) equal what the checked form would send, and only once. Anything else is aborted before it is sent. Bodies are compared as URL-encoded, multipart or flat JSON; any other shape is refused.
  • Page-level submit guard removed. The DOM checks before the click are now pre-checks only (docs(flow): name the layer that guarantees each threat #1598 THREAT_MODEL.md).
  • Late refusals halt. The run watches the wire check for 2 s after the click, then at the start of the next step and at the end of the run.
  • Tests submit for real. test_create_form_identity.py now posts to a server that records what arrives, so every case asserts what left the browser. Both create-form files pass: 32 tests.
  • CI fix. Anchor.form_identity was serialised as null into sealed content, so every bundle sealed earlier failed its digest. That caused the program-sections, registry and loop-showcase failures. The fix is a reviewed v2 omission plus a historical-seal check. All 26 CI failures pass locally. The 2 test_durable_authority_v13 failures were CI environment only; they pass on main and here.
  • docs/LIMITS.md now names the wire check as the safety layer.

#1589 is now stacked on this branch.

@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

AI re-review (coordinator): bc5791c

Verdict: the round-3 holes are closed at the network. I'm holding the merge on one coverage gap in the wire check, which I found by reading the code and have not reproduced. CI is also still running: flow core unit shard 1/2 is in progress.

Re-run probes (real headless Chromium, one worker)

  • PR's own test_create_form_wire.py, real server: 7/7 pass. It has sub7→sub9, bubble42, fd42 and fetch42, my round-3 holes rebuilt with a server that logs what arrives. All four are refused, and nothing reaches the server. plain, sub7 and fetch go out exactly once.
  • test_probe_1543.py, test_probe_1543_r2.py and test_probe_1581_r3.py, unmodified: 35 pass. These pages call preventDefault() and log in the page, so for the wire check they only cover the page-level pre-check, not what is sent. There:
    • every mismatch case halts (query swap, form= hidden field, token injection, file input, rot_wrong);
    • encounter_token=enc-…0001→…0002 now halts, because _RECORD_LIKE handles record-shaped values;
    • q_new→q_new and the other correct saves verify.
  • Arming is flaky under load. The same plain recording compiled armed in one run and unarmed in another; at load around 60, four bundles came out unarmed. An unarmed save is refused by qualification. So this is a false refusal of a correct save under CPU load, not a write that leaves. It isn't blocking, but a qualification run on a busy runner will hit it. store7 is unarmed every time, which matches the documented store-copy limit.

Wire check gaps (code reading against Playwright semantics; not reproduced)

  1. _arm_wire_check uses self.page.route. A page route covers this page and its frames only. A popup or new tab the page opens during the armed window is another Page, so its requests skip the check. A service-worker fetch also skips page routes unless the context blocks service workers. Use page.context.route (1589's _Browser already does), block service workers or refuse while one controls the page, and add a popup-plus-POST case to test_create_form_wire.py. I'd treat this as blocking until a test shows the popup write is refused. It is the one path I found that could end as an undeclared write while the run reports verified.
  2. WebSockets are not routed. A ws.send during the armed window is not checked. Add route_web_socket refusing while armed, or list it in LIMITS.md's wire section.
  3. Redirects. judge returns None for redirected_from, and Playwright doesn't route redirect hops anyway. A 307/308 from the declared POST re-sends the same body to the Location address, possibly on another origin. The server that took the declared write chooses that, so I don't count it as blocking. Still, LIMITS.md should say it.
  4. Unsafe GETs. GETs pass as reads, so an app with side-effecting GET endpoints (OpenEMR has some) is outside the check. That's by design; document it next to the wire check.
  5. Body parsing (not blocking):
    • Multipart parsing and multiset comparison look correct.
    • An extra field, a second submission and a mutate-then-resubmit are all refused.
    • Duplicate JSON keys are last-wins in json.loads, which can differ from a first-wins server. This only matters when the page sends the save as flat JSON.
    • A gzip body doesn't parse and is refused: safe.

Not merging. Once item 1 is closed with a test and CI is green, I'll merge.

From the #1581 re-review:
- The wire check routes on the browser context, so a popup or new tab the
  save opens is held too (real-Chromium test: its write never arrives).
- Service workers: the runner's browsers block them; a page one controls
  is refused before the click, and one that starts while armed halts.
- A WebSocket opened while the save is held is never connected; a message
  on one opened earlier halts the run.
- A JSON body with a repeated key is refused (two servers can read it two
  ways).
- The compiler treats a click on the typed form's submit button as a save
  whatever the screen did, so a slow recording still arms the check; when
  it can't be armed, the step says why.
- LIMITS.md lists what the wire check doesn't see: 307/308 redirects,
  side-effecting GETs, sockets opened earlier, late requests.
@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

AI re-review (coordinator): 5a4a25f

The popup and service-worker gap is closed. One WebSocket test fails locally, so auto-merge is on but will hold for CI.

  1. Routing. The wire check now routes the whole browser context (page.context.route), so popups and new tabs are covered.
    • A page that a service worker controls is refused before the click.
    • A service worker that starts during the save is reported.
    • The runner's own browser contexts block service workers.
  2. Tests (real Chromium, one worker, local server that counts received writes). 12 of 13 pass.
    • Popup: the popup's POST /account/close never arrives, and the run halts.
    • Service worker: the worker's fetch handler would carry the save past the route. The page is refused and nothing arrives. With service workers blocked, the same page saves once.
    • The four "changed after the check" rows are refused with nothing received. The three "goes out once" rows send exactly once.
    • test_wire_check.py: a repeated JSON key is refused, and the secret is masked in every encoding.
  3. The other checks:
    • Duplicate JSON keys: refused.
    • Unarmed compile: a form save is now always compiled irreversible, so an unarmed one is refused loudly by qualification. The test harness also asserts it is armed.
    • WebSockets: test_a_websocket_opened_by_the_save_halts_the_run fails twice in a row locally. The run reports success, so the WebSocket refusal never fired. Nothing is sent, because the test server has no WebSocket endpoint. So this is a detection gap, not a write that got out. The likely cause: the form's real POST navigates away before the WebSocket opens.
      • The test should hold the page, for example by sending the save with fetch, and use a server that accepts WebSockets, so it shows a frame being refused.
      • A frame sent on a WebSocket that was already open before arming is only reported after it is sent. Note that in LIMITS.md.

No blocking hole under the review rule. I'm enabling auto-merge, which waits for required CI. If the WebSocket test fails there too, it won't merge until that test is fixed.

@abrichr
abrichr enabled auto-merge October 28, 2026 22:40
@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Re-review fixes, at 5a4a25f97:

  • Popups (the blocking item). The wire check now routes on the browser context, so a popup or new tab the save opens is held too. New real-Chromium test: the click opens a popup whose page posts to /account/close. That write never reaches the server, and the run halts.
  • Service workers. The runner's own browsers (the replay launcher, run, the worklist pool, qualify) block service workers. A page that a service worker controls anyway is refused before the click, and a worker that starts while the save is held halts the run. New test: a page with a service worker is refused; with workers blocked, the same page saves once.
  • WebSockets. A socket opened while the save is held is never connected to the server. A message on a socket opened earlier halts the run, but it can't be stopped. New test: the save opens a socket and sends on it, and the run halts. Closing a socket from a sync route handler deadlocks Playwright, which is why the socket is left unconnected instead.
  • JSON bodies. A body with a repeated key is refused, with a unit test in test_wire_check.py.
  • LIMITS.md now lists what the wire check doesn't see: 307/308 redirects (the server's choice), side-effecting GETs, sockets opened earlier, and late requests.
  • Arming. The compiler treats a click on the typed form's own submit button as a save, whatever the screen did afterwards. That removes the "recorded reversible" path. When the check can't be armed on such a save, the step says why and a warning is logged. I couldn't reproduce an unarmed recording locally: 5 of 5 armed at load 38. So I can't promise this covers every flaky case. A recording whose page-side click evidence is missing entirely still compiles unarmed and is refused.

Locally: 566 compile, recorder, create-form and bundle tests pass, and 316 backend, replayer and qualify tests pass.

…ents

new_page now blocks service workers, and the backend watches WebSockets
for the wire check; the CLI and console fakes and the screenshot-mask
listener check predated both.
The wire check let a WebSocket the page opened while a create-form save was
held reach the server on CI (test_a_websocket_opened_by_the_save_halts_the_run).

Cause: CI runs Playwright 1.61.0. Its context.route_web_socket works by
replacing window.WebSocket through an init script, so it routes only sockets
of documents loaded after it is registered. The backend registered it lazily
on the first save, while the form's document was already open (the runner
hands the backend a page it has already navigated), so that document's
socket bypassed the route and connected. Playwright 1.64 also patches open
documents, which is why it passed locally. A second, timing-dependent hole:
the route connected any socket whose callback ran after disarm, so a socket
opened during the hold but serviced late would connect.

Fix: a page-side guard wraps the WebSocket constructor in every document of
the context (an init script registered when the backend is created, again
after Playwright's routing, and installed in each open frame when a save is
held). The hold is set page-side before the click (open frames, plus an init
script so documents loaded during the hold start held) and released only
after the outcome has been read. A socket constructed while held throws
SecurityError and is never created; the guard counts it and the refusal
reports it. Arming fails closed when a frame can't be held. The Python route
stays as a second layer.

Known limit (documented): a page script that kept a reference to the
browser's own constructor before the backend existed is outside the guard.

Tests: a socket opened while held never connects, including when the route
callback runs only after disarm (fails before the fix on 1.61 and 1.64), and
sockets connect again once the hold is released, in old, popup and new
documents.
…ld save

The review of 94adb72 found routes past the page-side guard:

- WebSocket.prototype.constructor and the wrapper's own prototype led back
  to the unguarded constructor (1.61 connected). Each prototype.constructor
  on the chain now points at the wrapper, whose prototype is EventTarget,
  and the static constants are copied instead of inherited.
- A dedicated (Blob) worker opened a WebSocket and delivered. Worker and
  SharedWorker are wrapped the same way, so one constructed while held is
  refused; a page already running a dedicated worker is refused before the
  click; a worker event during the hold is reported.
- A reference to the browser's WebSocket kept before the backend existed
  bypassed the guard. PlaywrightBackend.launch now installs the guard and
  the route before the first load (prepare_ws_guard, ws_guard=). For pages
  already open, the guard goes into open frames before Playwright's routing
  replaces WebSocket, so the browser's own prototype is guarded too and a
  message sent on such a socket while held is dropped (its handshake still
  happens: documented).
- Disarm releases the page side while the Python route still holds, then
  makes one more round trip to every frame before clearing it.

The review's probes are now tests in test_create_form_wire.py; against
94adb72 six fail on 1.61 and four on 1.64, and all pass with this commit.
@abrichr
abrichr merged commit 7a140fb into main Oct 29, 2026
44 checks passed
@abrichr
abrichr deleted the claude/create-form-identity-v2 branch October 29, 2026 05:51
abrichr added a commit that referenced this pull request Oct 29, 2026
…eceipts (#1589)

* feat: verified writes for any agent, and taught browser writes with receipts

openadapt verify: a write gets a verified receipt or a refusal, never an
unverified success.
- openadapt_flow.verify (also openadapt.verify): WriteContract,
  begin_write, verify_write and the verified_write context manager, a thin
  public face over verify_run's capture_baseline and verify_run. Any
  executor wraps its own write: a compiled program, an agent in any
  framework, a script or a person.
- `openadapt verify serve --contracts DIR` (and `openadapt-agent serve
  --verify DIR`): MCP tools verify_contracts, verify_begin and
  verify_check. The operator owns the contracts; the agent names one and
  the values it will write.

Taught writes: `serve --teach-browser --contracts DIR [--sign-in NAME=ENV]`.
A task that names a write contract may save to the taught site. Run 1 is
verified in the system of record before taught_<name> exists; every call
makes the write and returns done with a receipt, or needs_review with
REFUTED or RECONCILIATION_REQUIRED. Sign-in values come from the server's
environment and never reach the agent, a result or the program. Controls
in named iframes are listed and found through their frame, and a goto
action stays on the taught site.

The example app gains a sign-in, a New Order form and /api/orders to read
orders back. Quickstart: packages/flow/docs/VERIFY_WRITES.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: link #1589 in CAPABILITIES.md

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: taught writes go through the shared wire check; sealed receipts

From the #1589 review:
- A taught write's requests are pinned (method, path and the digest of its
  field names). Any other write is aborted before it is sent, while
  teaching and on every call, and the call ends in needs_review.
- Sign-in values never travel in an address and never reach the agent in
  any encoding (raw, percent, form, JSON, HTML, base64). They go only into
  a password field or a field of a POST sign-in form.
- Verified receipts are sealed with this computer's Seal key and say the
  values were declared by the agent and whether an operator's
  authorization was bound.

browser_teach now uses openadapt_flow.wire_check (from #1581) instead of
its own copy of the route guard, masking and body shapes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: say when a receipt is unsealed; write-lane button label

From the #1589 re-review: a verified result now carries seal (sealed, off,
or unsealed with why the key couldn't be used) instead of a silent null
seal_path, and the write lane lists POST buttons as sending only the
contract's writes rather than refused.

* fix: the teach browser blocks service workers

Their requests skip context routes, so the wire check couldn't see them.

* test: emit's no-mcp-import check starts without mcp loaded

Another test in the same process (the verify MCP tools) imports mcp, so
the check now clears it first and sees only what emission imports.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
abrichr added a commit that referenced this pull request Oct 29, 2026
Lands the verifier half of #1628 on its own, ahead of #1581 and #1589:
openadapt_flow.verifier (begin_write/verify_write, CONFIRMED/REFUTED/UNCERTAIN
with a Scope on every result) and write_ledger (append-only, HMAC-keyed,
leased per write key, consulted inside begin_write). #1588's capture() keeps
working, deprecated. The openadapt_flow.verify wrapper, the CLI and the MCP
tools come with #1589.
abrichr added a commit that referenced this pull request Oct 29, 2026
…ble uncertain-write ledger (#1628)

* frame: key audit entries by a record field, and declare side records

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: diff the whole database, with a reviewed allow-list

frame.database snapshots every table and column before and after the save
(three statements, whatever the table count) and judges each table as a
slice. A row added, removed or changed is collateral unless a declared
effect, a side record or an attributed concurrent writer explains it. A new
declared or side row that sets a column nothing declares is collateral too.
Tables over max_rows are fingerprinted. allow_tables, reviewed per app,
replaces the read-list; an unreadable snapshot leaves the frame not checked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: read rows wider than one JSON call, keeping null columns

SQLite caps a function at 127 arguments and OpenEMR's chart has more
columns, so the snapshot failed and the frame was not checked. Each row is
now read as JSON objects of at most 50 columns, merged in Python (a JSON
merge patch would drop null values).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: a default the database computes at insert is a default

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: audit attribution can name a table's record through another field

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* verifier: a stable Python API for checking one write

openadapt_flow.verifier: capture() reads the system of record before the
write, verify_write() judges it against the same contract and returns
verified, the outcome, which parts were checked, plain reasons, the receipt
and its evidence. It wraps verify_run and checks exactly what it checks; a
snapshot verifies one write. A test pins the public names and signatures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat: verified writes for any agent, and taught browser writes with receipts

openadapt verify: a write gets a verified receipt or a refusal, never an
unverified success.
- openadapt_flow.verify (also openadapt.verify): WriteContract,
  begin_write, verify_write and the verified_write context manager, a thin
  public face over verify_run's capture_baseline and verify_run. Any
  executor wraps its own write: a compiled program, an agent in any
  framework, a script or a person.
- `openadapt verify serve --contracts DIR` (and `openadapt-agent serve
  --verify DIR`): MCP tools verify_contracts, verify_begin and
  verify_check. The operator owns the contracts; the agent names one and
  the values it will write.

Taught writes: `serve --teach-browser --contracts DIR [--sign-in NAME=ENV]`.
A task that names a write contract may save to the taught site. Run 1 is
verified in the system of record before taught_<name> exists; every call
makes the write and returns done with a receipt, or needs_review with
REFUTED or RECONCILIATION_REQUIRED. Sign-in values come from the server's
environment and never reach the agent, a result or the program. Controls
in named iframes are listed and found through their frame, and a goto
action stays on the taught site.

The example app gains a sign-in, a New Order form and /api/orders to read
orders back. Quickstart: packages/flow/docs/VERIFY_WRITES.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: link #1589 in CAPABILITIES.md

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* verifier: actor kinds, refuted vs unconfirmed, receipt hash, database diff

From the MCP skills, RL reward and undo sessions:
- Actor(kind, id, build): the receipt records who wrote (program, agent,
  script, person);
- WriteResult.status: refuted (the store shows a different, extra or
  duplicate write) vs unconfirmed (it doesn't show the write, or couldn't
  be read: reconcile before retrying), and receipt_sha256;
- capture() raises BaselineUnreadable when the store can't be read, so a
  caller can stop before writing; write_attempted passes through;
- capture_database() and diff_database(): a whole-database snapshot and a
  per-table diff (rows added, removed, changed, column names, no values),
  over the database's own driver.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: whole-database diff without values, in one consistent read

From the adversarial review of #1582:
- PHI: the baseline keeps keyed MACs (BLAKE2b with the run salt) per cell
  and per row key, and keyed ids of audit entries; plain values are read only
  for rows the save added, in memory.
- Consistency: one connection, one read-only transaction (START TRANSACTION
  WITH CONSISTENT SNAPSHOT, READ ONLY; BEGIN on SQLite), with the audit log
  read inside it when it shares the connection.
- Large tables: salted SHA-256 per bucket of 1024 keys up to the baseline's
  largest key, added rows read in full, so a clean save on a table over
  max_rows holds. A table keeps its baseline representation after the save.
- Silent passes closed: a value is a default only if it equals the declared
  one; a clock default must fall between the snapshots' database clocks;
  ignore_fields apply to surfaces and database.ignore_columns to tables;
  sqlite_sequence and AUTO_INCREMENT catch a write then delete; table
  definitions and triggers, views, routines and events are digested; only a
  single generated key may hold any value; with frame.database, only a write
  event on the same table attributes a change (event_field, write_events,
  insert_events, table_field).
- Allow-listed tables are append-only unless skipped with a reason; every
  visible schema is read; settle_s delays the read for late writes; the
  receipt counts tables compared, append-only and skipped and binds the
  allow-list digest.
- Another actor's later write to the bot's new row, in a column the save
  doesn't declare, is concurrent when the bot only inserted it; a bot write
  to that column still fails. A surface that is a view of a snapshotted table
  is judged through the table.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: excuse another actor's change to a declared record only through a write to its table, and never on a record the bot also wrote

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: a DEFAULT NULL column (shown as the text NULL) has no default

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: a side row of another actor's new record is theirs; report their additions by surface id

OpenEMR logs a contact row with no patient, so no write event attributes it;
its link to a chart another user created in the window does, the same way
side records attach to the bot's chart. FrameVerdict.added_by_others names
such records as their surface does, for checks that read the surface.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: taught writes go through the shared wire check; sealed receipts

From the #1589 review:
- A taught write's requests are pinned (method, path and the digest of its
  field names). Any other write is aborted before it is sent, while
  teaching and on every call, and the call ends in needs_review.
- Sign-in values never travel in an address and never reach the agent in
  any encoding (raw, percent, form, JSON, HTML, base64). They go only into
  a password field or a field of a POST sign-in form.
- Verified receipts are sealed with this computer's Seal key and say the
  values were declared by the agent and whether an operator's
  authorization was bound.

browser_teach now uses openadapt_flow.wire_check (from #1581) instead of
its own copy of the route guard, masking and body shapes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: the MySQL snapshot sends no statement that can fail

A driver may end the session on an error, so the snapshot checks for MySQL
8's information_schema_stats_expiry before setting it, instead of trying it
and catching MariaDB's error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: rows by key in every table, attribution by kind and count, MAC key outside the baseline

From the second review of #1582:
- Every table keeps, per row, a 128-bit MAC of its key and of its contents
  (without ignore_columns); cell MACs for all columns up to max_rows (now
  2000), and for the reference columns above it. A changed row in a large
  table goes through attribution by its key; ignore_columns hold; an added
  row is found by key for any primary key. Buckets are gone.
- Attribution: the record is named as it was before the change; an added
  row needs an insert entry, a removed one a delete entry (delete_events), a
  changed one an update; each entry explains at most one row; a row whose
  record key changed is never explained. The log's own limits are documented.
- MACs: BLAKE2b, 128 bits, bound to table and column, keyed by the salt and a
  host secret (OPENADAPT_FRAME_SECRET, or an owner-only key file) that is
  never in the baseline. Audit-entry ids use the same key.
- MySQL: AUTO_INCREMENT is read just before the consistent snapshot starts,
  on the column the database numbers whether or not it is the key (the live
  run's false hold on patient_data); no GROUP_CONCAT; the frame is not
  checked when the read account can't see triggers (it needs TRIGGER); a
  required schema the account can't see is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* verifier: the database snapshot's max_rows default follows the frame (2000)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* app profiles: one versioned OpenEMR bookkeeping profile for the frame, rollback and RL

openadapt_flow.app_profiles.load_profile('openemr'): the tables and columns
OpenEMR writes on its own (logs and id maps append-only; per-session state
skipped with a reason; uuid backfill and the users ON UPDATE stamp as column
rules), from the RL environment's calibration (2026-10-28.1) and the frame's
live witness. No rule may name a patient-data table except for a system
column. frame.database.profile applies it; the receipt names the version,
its digest and the rules that excused something in the run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* verifier: capture_database takes the app profile the frame uses

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* app profiles: contact is append-only bookkeeping (openemr-frame-profile/2026-10-28.2)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: tests use their own host secret; document that a deployment keeps it from the agent

The test suite created the real ~/.config/openadapt/frame.key; an autouse
fixture now sets OPENADAPT_FRAME_SECRET per test. The default key file only
keeps the secret out of the baseline, since a same-user agent can read it:
a deployment sets the secret from a store the agent can't read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* app profiles: compare credential tables; backfills only; an allowlist; a pinned digest

From the third review of #1604:
- No table is skipped any more. Credentials and access are compared:
  users_secure and login_mfa_registrations excuse only sign-in counters and
  timestamps on the acting user's own row; keys are append-only;
  background_services excuses only next_run and running; user_settings and
  recent_patients only the acting user's own rows (owner by user id, looked
  up in the snapshot).
- uuid rules excuse a backfill only, from NULL to a value; users.last_updated
  only beside one.
- The patient-data denylist is now an allowlist: a rule may name only a
  reviewed bookkeeping table (lower-cased, unqualified); '*' only a system
  column backfill.
- Profiles load only from the package, by name; frame.database.profile_sha256
  pins the digest (bound into frame_sha256), and a mismatch is refused.
- rules_used lists only the rules that excused something in the run.
- A table without a primary key keys its rows without the columns a rule
  lets move. The profile is registered in public-artifacts.json.

Profile version openemr-frame-profile/2026-10-29.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* verifier: capture_database takes before= and actor_names; diff_database applies the profile

Rollback needs the plain values of rows that changed after a run began
(before=, in memory only), and the same judgment the frame uses: with
profile= and the acting user's names, diff_database leaves out a row change
the profile calls the app's own (the user's sign-in counters, a uuid
backfill), so a clean undo isn't reported dirty.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* frame: the host secret is created whole and race-free on first use

Two processes starting together could read a half-written key file and
MAC the baseline and the read after it with different keys. The secret is
now written to a private temp file and linked into place; the loser reads
the winner's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* app profiles: a colleague who wrote in the window owns their per-user UI state

own_rows rules (user_settings, recent_patients) also excuse the rows of
another user with a write entry in the window: their own work. Credential
column rules stay on the acting user's row. The users table's ids and names
are read in the snapshot, in memory only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* app profiles: keys and MFA registrations are compared in full (2026-10-29.2)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: say when a receipt is unsealed; write-lane button label

From the #1589 re-review: a verified result now carries seal (sealed, off,
or unsealed with why the key couldn't be used) instead of a silent null
seal_path, and the write lane lists POST buttons as sending only the
contract's writes rather than refused.

* fix: the teach browser blocks service workers

Their requests skip context routes, so the wire check couldn't see them.

* frame: narrow types for mypy (renames only, no behaviour change)

* docs: refresh llms-full.txt for the verifier API row

* test: emit's no-mcp-import check starts without mcp loaded

Another test in the same process (the verify MCP tools) imports mcp, so
the check now clears it first and sees only what emission imports.

* frame: record_key refs never excuse what they don't name

Adversarial review of #1570 found three silent passes and one more held:
- a row whose record_key changed (a note refiled to another patient) was
  matched by its new value only; it now names neither and is collateral;
- a bot entry was set aside when its ref equalled a moved row's id; moved
  rows are now compared by their record_key value;
- a declared record with no record_key value added "None" to the declared
  set, excusing every bot entry without a patient; the frame is now
  not_checked;
- a side record linked to an existing record's id was part of the save; a
  link already in the baseline is no longer a new record's.
Nothing changes when record_key and side_records are unset.

* verifier: a snapshot is single use across reloads and copies; errors are VerifierError

Adversarial review of #1588: verify_write copied each snapshot to a fresh
temp file, so Snapshot.load twice, a payload copy, or a baseline the CLI had
already used each verified a second write (an RL replay hole). A process-wide
set of used salts now refuses copies, and a loaded snapshot verifies through
its own file, so verify-run's .used marker applies. VerifyRunError (unbound
params, another contract's snapshot) now surfaces as VerifierError, the
docstring says a refused call uses up the snapshot, and repr hides the
payload.

* test: plain setup for the other-contract snapshot case

* frame: a side row must be filed under its parent's record_key

Re-review of #1570: with link_field other than record_key, a side row
linked to the new chart but filed under another patient was part of the
save, and its record_key value excused that patient's bot log entries.
It is now an ordinary unexplained addition.

* frame: a side row with no record_key value can still be its parent's

* verifier: single use survives copies and other processes

Re-review of #1588: the used-salt set lived in one process, so a copied
file, a load-save-load to a new path, or a snapshot pickled to a worker
verified a second write in a new process. A use is now also recorded as an
O_EXCL file named by the salt's digest in a private per-user directory,
checked on every call. A missing salt, a non-dict payload and an OSError
raise VerifierError with their own messages; the docstring says an
unconfirmed result also uses up the snapshot. Snapshot's fields and
load/save signatures are pinned.

* frame: with record_key, side records link by it; only collateral excuses

Re-review round 3 of #1570 and of its port to #1582:
- the parent-ref rule let an extra side row filed under another patient
  fall through to attribution, where a colleague's entry excused it. With
  record_key set, a side record must now link by record_key on both sides
  (validated), so a side row's link and its record_key are the same value;
  the parent-ref rule is gone.
- moved records compared by record_key let another user's concurrent change
  to a patient excuse the bot's own entry for that patient (a write the
  frame doesn't read). Only collateral records excuse entries now.

* frame: at most count side rows per link of another actor's new chart are theirs

Re-review of the #1570 port, round 2: a side row linked to a colleague's new
chart was theirs with no count, so the bot's extra contact or history row
for that chart ended held. Rows beyond the side record's count are now
unattributed collateral.

* verifier: capture issues a record, verify removes it

Re-review round 3 of #1588: the used-salt marker lived under the caller's
HOME, so a worker with another HOME (or none, another user, container or
host) replayed a used snapshot. capture() now records each snapshot as
issued (an O_EXCL file named by the digest of its salt bytes) and
verify_write() removes it; only one caller can, and a snapshot with no
record is refused, so another state directory fails closed instead of
replaying. Markers no longer pile up. OPENADAPT_VERIFIER_STATE_DIR gives
every worker one directory; it must be a private directory owned by the
user. The salt is keyed by its bytes, not its hex case, and the in-memory
payload whose salt was used is what's judged.

* verifier: the state root is private too; document capture-once and the state dir

Re-review round 4 of #1588: check the state root as well as issued/ (T6),
and say in the module docstring that the judged writer must not reach the
state directory (T5), that a snapshot verifies only under the state dir it
was captured under, that the caller captures once per write (T4), and that
an unverified capture leaves its record (T7).

* docs(flow): one verifier API and the uncertain-write ledger (design)

* feat(flow): one verifier API with three outcomes, and a durable uncertain-write ledger

openadapt_flow.verifier is the one public API for checking a write:
begin_write(contract, params, actor=...) then verify_write(pending). Every
result has an outcome (CONFIRMED, REFUTED or UNCERTAIN) and a scope: the
read path, the stores read, the time window, and what it doesn't cover.
openadapt_flow.verify (#1589) is a deprecated wrapper with the same
signatures; #1588's capture/Snapshot is replaced.

begin_write consults an append-only ledger keyed by the write's identity
(an HMAC over the contract, its bound values and an optional write key)
before anything is sent. A flock lease shows whether another worker holds
the write; a free lease on an open attempt means its owner died between
the send and the read-back, and reconciliation judges it against its own
baseline before any resend. REFUTED and UNCERTAIN attempts stay held until
a read-back resolves them or a person releases them
(openadapt verify ledger list, reconcile, release).

* verifier: delivery is the caller's call, per-value declared_by, isinstance kept in the wrapper

* verifier: fix the adversarial review's findings

- B1: key a write on Contract.identity (effects, unbound, no read settings),
  so a contract edit between a crash and a restart can't drop the hold.
- B2: a landed write without a write key is handed back to an identical call
  for repeat_s (10 min); verify_begin takes an optional write_key.
- B3: reconciliation never confirms when another attempt with the same values
  began after this one.
- B4: a frame breach that other ledger attempts in the window explain gives
  UNCERTAIN with landed=true (not held, never resent), not REFUTED.
- S1: an attempt's values and baseline are removed once it resolves for good.
- S2: ledger failures raise WriteHeld; S3: a released pending write doesn't
  record; S4: browser_teach checks for sends after closing; S5: values are
  normalized before keying.

* verifier: second review round

- N1: Contract.identity includes the store (verifier kind and location,
  environment_id, no credentials, loopback normalized).
- N2/N3: verify_write and reconcile never confirm when another attempt with
  the same values was open during the window (sub-second ledger times; the
  same key's predecessors are excluded, the lease orders them).
- N4: only attempts on the same store that landed in the window, or hold a
  live lease, excuse added records; the result is landed + review and stays
  listed for a person.

* verifier: third review round

- R1: a released attempt is free, also when it was still open.
- R2: released attempts are closed for the overlap checks; an open attempt
  counts while its owner is alive or for the contract's window.
- R3: a landed write under review is handed back until a person releases it,
  so a later attempt never hides it from the list.
- R4: store addresses are normalized (scheme, host case, default port,
  trailing slash); the same effects and values open under another address
  hold the write.
- *_env names stay in the store identity (tenants by credential); the CLI
  list shows the repeat-window state and review.

* verifier: fourth review round

- R5: the same write under another address for the store is held whatever
  its state (in flight, unresolved or made), never resent.
- R6: that hold applies only within one verifier kind, environment_id and
  host, so another environment is never held.
- An attempt released before any outcome never counts as overlapping.
- release takes --found absent|landed; landed hands the write back for good.

* verifier: fifth review round

- Contract.shape includes SQL server host, port and database names, and
  *_env credential names, so two servers or tenants never hold each other.
- Ledger.release requires found=absent|landed.
- Document the host-alias limit and per-store environment_id.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
abrichr added a commit that referenced this pull request Oct 29, 2026
* feat: verified writes for any agent, and taught browser writes with receipts

openadapt verify: a write gets a verified receipt or a refusal, never an
unverified success.
- openadapt_flow.verify (also openadapt.verify): WriteContract,
  begin_write, verify_write and the verified_write context manager, a thin
  public face over verify_run's capture_baseline and verify_run. Any
  executor wraps its own write: a compiled program, an agent in any
  framework, a script or a person.
- `openadapt verify serve --contracts DIR` (and `openadapt-agent serve
  --verify DIR`): MCP tools verify_contracts, verify_begin and
  verify_check. The operator owns the contracts; the agent names one and
  the values it will write.

Taught writes: `serve --teach-browser --contracts DIR [--sign-in NAME=ENV]`.
A task that names a write contract may save to the taught site. Run 1 is
verified in the system of record before taught_<name> exists; every call
makes the write and returns done with a receipt, or needs_review with
REFUTED or RECONCILIATION_REQUIRED. Sign-in values come from the server's
environment and never reach the agent, a result or the program. Controls
in named iframes are listed and found through their frame, and a goto
action stays on the taught site.

The example app gains a sign-in, a New Order form and /api/orders to read
orders back. Quickstart: packages/flow/docs/VERIFY_WRITES.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: link #1589 in CAPABILITIES.md

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: taught writes go through the shared wire check; sealed receipts

From the #1589 review:
- A taught write's requests are pinned (method, path and the digest of its
  field names). Any other write is aborted before it is sent, while
  teaching and on every call, and the call ends in needs_review.
- Sign-in values never travel in an address and never reach the agent in
  any encoding (raw, percent, form, JSON, HTML, base64). They go only into
  a password field or a field of a POST sign-in form.
- Verified receipts are sealed with this computer's Seal key and say the
  values were declared by the agent and whether an operator's
  authorization was bound.

browser_teach now uses openadapt_flow.wire_check (from #1581) instead of
its own copy of the route guard, masking and body shapes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat(flow): verified write tools for agent frameworks

verified(tool, contract=...) wraps a Claude Agent SDK, OpenAI Agents SDK,
LangGraph or CrewAI write tool. The tool keeps its name and arguments; each
call reads the system of record before and after (openadapt_flow.verify)
and returns a receipt or a refusal. The contract and pinned values come from
the operator, never the agent's arguments, and a refused write is held
instead of retried. WriteGuard does the same without a framework.

Each adapter is tested in its framework's own agent loop with a scripted
model, writing through the example app's order form over HTTP (verified,
dropped, saved twice), plus an opt-in real-model test. A new CI job runs
them from the lock with every framework extra.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(flow): CrewAI stays outside the lock; test_emit tolerates an imported mcp

Every CrewAI release pins a chromadb with open critical advisories (no
patched version), and current releases need mcp<2 and openai<3, so the
crewai extra is gone; the adapter works with the crewai a user installs,
and the frameworks CI job installs crewai==1.6.1 beside the lock. The
lock now changes only openai (2.45.0 to 3.40.0, for openai-agents).

test_emit_mcp_server asserted that mcp was not in sys.modules, which
fails when an earlier test file on the same worker imported mcp; it now
clears mcp first, so it still checks that emission doesn't import it.

* fix: say when a receipt is unsealed; write-lane button label

From the #1589 re-review: a verified result now carries seal (sealed, off,
or unsealed with why the key couldn't be used) instead of a silent null
seal_path, and the write lane lists POST buttons as sending only the
contract's writes rather than refused.

* fix: the teach browser blocks service workers

Their requests skip context routes, so the wire check couldn't see them.

* fix(flow): a cancelled or timed-out write is read back and not sent again

Review of #1609:
- WriteGuard.arun caught Exception, so a cancellation (the OpenAI Agents
  SDK's tool timeout, asyncio.timeout) skipped the read-back and the
  hold; the retry wrote again and was reported verified. A write that may
  have been sent is now always read back, also on cancellation and on
  KeyboardInterrupt, and an identical retry gets that first result
  (not_sent_again) instead of a second write.
- A waiter cancelled while queued for the guard's lock could take it later
  and never release it. The lock is now polled without a blocking thread
  and released in finally.
- Replies list who declared each value (values_declared_by: operator or
  agent), and the split enters the receipt's evidence as the executor
  build digest. The guide says what verified proves and doesn't.
- claude_sdk.only() builds options with every built-in tool off and only
  the given servers allowed; the guide says to wrap every tool that can
  write. The CrewAI note and the CI comment name the chromadb advisories.

Regression tests: the SDK's own timeout (the review's probe), a timeout
in asyncio, an interrupted sync write, a cancelled lock waiter, and the
declared-by split in replies and evidence.

* fix(flow): a read-back queued behind a full pool still runs on a timeout

Round 2 review of #1609: the read-back ran on the guard's thread pool
through asyncio.wrap_future, which cancels a job that hasn't started when
the caller is cancelled. A timeout with the pool full then skipped the
read-back and the hold, and the retry wrote again (2 orders from 2 POSTs).
Every pool job is now shielded; on cancellation a job that hasn't started
is taken back and run inline, and one that has is waited for, so the
read-back (and the first read's cleanup) always happens.

Tests: a timeout while the read-back waits behind a full pool (1 POST,
the retry gets the first result), and while the first read waits (nothing
written). The first fails on the previous commit.

* test: emit's no-mcp-import check starts without mcp loaded

Another test in the same process (the verify MCP tools) imports mcp, so
the check now clears it first and sees only what emission imports.

* ci(frameworks): install CrewAI from a hashed requirements file (main's rule: every CI install needs hashes)

The same 122 packages as the unhashed install resolved in CI: openai 3.3.0,
jiter 0.14.0 and rich 14.3.4 on top of the lock, as before.

* agents: a call cancelled after the baseline but before the write closes its ledger attempt as never sent

With #1628's write ledger on main, deleting the baseline left the attempt open,
so an identical retry was held as possibly still landing.

* agents: the never-sent close carries the guard's executor and is never sealed or raised; a ledger AlreadyWritten returns the earlier result

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
abrichr added a commit that referenced this pull request Oct 29, 2026
)

* feat: verified writes for any agent, and taught browser writes with receipts

openadapt verify: a write gets a verified receipt or a refusal, never an
unverified success.
- openadapt_flow.verify (also openadapt.verify): WriteContract,
  begin_write, verify_write and the verified_write context manager, a thin
  public face over verify_run's capture_baseline and verify_run. Any
  executor wraps its own write: a compiled program, an agent in any
  framework, a script or a person.
- `openadapt verify serve --contracts DIR` (and `openadapt-agent serve
  --verify DIR`): MCP tools verify_contracts, verify_begin and
  verify_check. The operator owns the contracts; the agent names one and
  the values it will write.

Taught writes: `serve --teach-browser --contracts DIR [--sign-in NAME=ENV]`.
A task that names a write contract may save to the taught site. Run 1 is
verified in the system of record before taught_<name> exists; every call
makes the write and returns done with a receipt, or needs_review with
REFUTED or RECONCILIATION_REQUIRED. Sign-in values come from the server's
environment and never reach the agent, a result or the program. Controls
in named iframes are listed and found through their frame, and a goto
action stays on the taught site.

The example app gains a sign-in, a New Order form and /api/orders to read
orders back. Quickstart: packages/flow/docs/VERIFY_WRITES.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: link #1589 in CAPABILITIES.md

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: taught writes go through the shared wire check; sealed receipts

From the #1589 review:
- A taught write's requests are pinned (method, path and the digest of its
  field names). Any other write is aborted before it is sent, while
  teaching and on every call, and the call ends in needs_review.
- Sign-in values never travel in an address and never reach the agent in
  any encoding (raw, percent, form, JSON, HTML, base64). They go only into
  a password field or a field of a POST sign-in form.
- Verified receipts are sealed with this computer's Seal key and say the
  values were declared by the agent and whether an operator's
  authorization was bound.

browser_teach now uses openadapt_flow.wire_check (from #1581) instead of
its own copy of the route guard, masking and body shapes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat(flow): verified write tools for agent frameworks

verified(tool, contract=...) wraps a Claude Agent SDK, OpenAI Agents SDK,
LangGraph or CrewAI write tool. The tool keeps its name and arguments; each
call reads the system of record before and after (openadapt_flow.verify)
and returns a receipt or a refusal. The contract and pinned values come from
the operator, never the agent's arguments, and a refused write is held
instead of retried. WriteGuard does the same without a framework.

Each adapter is tested in its framework's own agent loop with a scripted
model, writing through the example app's order form over HTTP (verified,
dropped, saved twice), plus an opt-in real-model test. A new CI job runs
them from the lock with every framework extra.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat(flow): skill cache for framework agents

A framework agent (LangGraph, OpenAI Agents, CrewAI, Claude Agent SDK) does
run 1 through compile-agent's harness as plain tools (BrowserTools); when
the system of record shows exactly the task's record, the operator is
offered the proposed checks, canaries run, and the admitted program is
emitted as an MCP server. The next matching task calls that server's tool
over stdio: no model calls, every save read back, a verify-run receipt.

Browser actions from frameworks that run tools on worker threads are
served on the browser's own thread.

The emitted MCP server imported mcp.server.fastmcp, which mcp 2 (the
locked version) removed, so every emitted server failed to start; it now
imports MCPServer and falls back to FastMCP on mcp 1. The new e2e test
starts the server, which the existing one only parsed.

* fix(flow): an input submit button is a button named by its value

The structural locator mapped every <input> to role textbox and took
its name attribute, so <input type=submit name=submit value="Create
ticket"> (Trac's New Ticket) was recorded as textbox "submit" and replay
could not find it. Input buttons are now role button, named by value
(alt for an image input), as the accessibility tree and get_by_role
name them.

* feat(flow): BrowserTools can scroll; the measurement script

The structural listing shows what is on screen, so an agent without a
scroll tool could reach a button below the fold only with a key press.
benchmark/skill_cache/measure.py runs a LangGraph agent's run 1 through
the cache and later runs through the compiled skill's MCP server.

* bench(flow): skill cache measurement on MockMed, with an independent store check

* bench(flow): skill cache rows name the trial, engine commit and model

* docs(flow): skill cache guide, MockMed measurement, Trac findings

MockMed, LangGraph on claude-sonnet-5-5, 3 trials of 10 runs: run 1
(agent, record check, canary, admission) 6 model calls, $0.022, 23.5 s
median; runs 2 to 10 through the compiled skill's MCP server 27/27
VERIFIED, also confirmed by a separate store read, 0 model calls, 7.8 s
median. Trac 1.6: run 1 verified 7 of 12 attempts, none admitted; the
engine gaps that stopped each are listed. OpenEMR not measured.

* fix(flow): CrewAI stays outside the lock; test_emit tolerates an imported mcp

Every CrewAI release pins a chromadb with open critical advisories (no
patched version), and current releases need mcp<2 and openai<3, so the
crewai extra is gone; the adapter works with the crewai a user installs,
and the frameworks CI job installs crewai==1.6.1 beside the lock. The
lock now changes only openai (2.45.0 to 3.40.0, for openai-agents).

test_emit_mcp_server asserted that mcp was not in sys.modules, which
fails when an earlier test file on the same worker imported mcp; it now
clears mcp first, so it still checks that emission doesn't import it.

* docs(flow): name where the measured engine commit lives

* fix: say when a receipt is unsealed; write-lane button label

From the #1589 re-review: a verified result now carries seal (sealed, off,
or unsealed with why the key couldn't be used) instead of a silent null
seal_path, and the write lane lists POST buttons as sending only the
contract's writes rather than refused.

* fix: the teach browser blocks service workers

Their requests skip context routes, so the wire check couldn't see them.

* fix(flow): a cancelled or timed-out write is read back and not sent again

Review of #1609:
- WriteGuard.arun caught Exception, so a cancellation (the OpenAI Agents
  SDK's tool timeout, asyncio.timeout) skipped the read-back and the
  hold; the retry wrote again and was reported verified. A write that may
  have been sent is now always read back, also on cancellation and on
  KeyboardInterrupt, and an identical retry gets that first result
  (not_sent_again) instead of a second write.
- A waiter cancelled while queued for the guard's lock could take it later
  and never release it. The lock is now polled without a blocking thread
  and released in finally.
- Replies list who declared each value (values_declared_by: operator or
  agent), and the split enters the receipt's evidence as the executor
  build digest. The guide says what verified proves and doesn't.
- claude_sdk.only() builds options with every built-in tool off and only
  the given servers allowed; the guide says to wrap every tool that can
  write. The CrewAI note and the CI comment name the chromadb advisories.

Regression tests: the SDK's own timeout (the review's probe), a timeout
in asyncio, an interrupted sync write, a cancelled lock waiter, and the
declared-by split in replies and evidence.

* fix(flow): a skill serves only its own task and record, and is re-checked

Review of #1613:
- The cache key was the task id plus parameter names, so another
  patient's task ran Jane's program, saved Alex's note on Jane's chart
  and reported VERIFIED. The key is now a digest of the instruction,
  parameter names, expect, on_screen, url, the verify.yaml contents, the
  policy and an operator-named environment; the full key is stored with
  the skill and compared before it is served.
- Every skill run is also checked against the current task in the
  system of record (compile-agent's run-1 record check: one new record
  with this task's expect fields and values), so a program served by a
  collision fails as RECORD_CHECK_FAILED instead of passing on its
  compile-time checks.
- A skill run that fails either check is evicted (kept under
  <cache_dir>/evicted/), so the next run recompiles through the agent;
  SkillCache.evict does it by hand. cache_dir defaults to
  ~/.openadapt/skills. SkillRun.wire_checked says whether the writes were
  checked before they left (no, on the skill path), and the guide lists
  the layers a skill run has.
- Per-run measurement rows moved to the private corpus
  (OpenAdaptAI/openadapt-corpus#19); the public tree keeps the summary,
  and measure.py writes rows to its work folder. The README is scoped to
  what was measured (repeats on the same record) and names no internal
  branches or process.

Regression tests: the review's probe (Alex's task goes to the agent and
lands on p2), a forced key collision (RECORD_CHECK_FAILED, evicted), and
the per-user cache dir and key fields.

* fix(flow): a read-back queued behind a full pool still runs on a timeout

Round 2 review of #1609: the read-back ran on the guard's thread pool
through asyncio.wrap_future, which cancels a job that hasn't started when
the caller is cancelled. A timeout with the pool full then skipped the
read-back and the hold, and the retry wrote again (2 orders from 2 POSTs).
Every pool job is now shielded; on cancellation a job that hasn't started
is taken back and run inline, and one that has is waited for, so the
read-back (and the first read's cleanup) always happens.

Tests: a timeout while the read-back waits behind a full pool (1 POST,
the retry gets the first result), and while the first read waits (nothing
written). The first fails on the previous commit.

* fix(flow): another user's write elsewhere doesn't fail a skill run

Round 2 review of #1613: the skill path's record check required exactly
one new record in the whole store, so another user saving on another
chart during the run turned a correct save into RECORD_CHECK_FAILED and
evicted the skill. task_record_check attributes to the task only the
records that match its expect fields: exactly one new one, with the
task's values, and no change to that record's existing rows. Writes to
other records are counted (record_check["outside_run"]), not refused; a
write the program itself made elsewhere still leaves this task's record
unwritten, and the program's own receipt covers the frame.

Tests: a note saved on p2 while the skill saves on p1 (verified, counted
outside the run, not evicted); the forced key collision still fails, with
0 records of the task's record.

* test: emit's no-mcp-import check starts without mcp loaded

Another test in the same process (the verify MCP tools) imports mcp, so
the check now clears it first and sees only what emission imports.

* ci(frameworks): install CrewAI from a hashed requirements file (main's rule: every CI install needs hashes)

The same 122 packages as the unhashed install resolved in CI: openai 3.3.0,
jiter 0.14.0 and rich 14.3.4 on top of the lock, as before.

* agents: a call cancelled after the baseline but before the write closes its ledger attempt as never sent

With #1628's write ledger on main, deleting the baseline left the attempt open,
so an identical retry was held as possibly still landing.

* agents: the never-sent close carries the guard's executor and is never sealed or raised; a ledger AlreadyWritten returns the earlier result

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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