Skip to content

feat(attended): let an operator reject a halt and end the run - #295

Merged
abrichr merged 6 commits into
mainfrom
feat/attended-reject-action
Jul 28, 2026
Merged

feat(attended): let an operator reject a halt and end the run#295
abrichr merged 6 commits into
mainfrom
feat/attended-reject-action

Conversation

@abrichr

@abrichr abrichr commented Jul 28, 2026

Copy link
Copy Markdown
Member

Adds reject — an explicit "Stop, this is wrong" that terminates the run — to the attended-decision vocabulary. Second of four repos; depends on openadapt-types 0.6.3 (merged, published, verified on PyPI).

Why

Today's actions are continue / skip / teach / escalate. When the engine is correct to halt — the record genuinely says Ineligible, the member genuinely is not eligible — the operator's only one-tap options are to assert they fixed something or to hand it to a colleague. There is no way to say "you were right, stop."

  • Disagreement has no channel, so the answer distribution can never tell you how often your halts are correct — data you need before tuning halt rates.
  • Its visible presence communicates that disagreement is an expected outcome.
  • It removes the situation where the only one-tap resolution is the agreeable one, which is the shape that produces rubber-stamping.

escalate is not relabelled to carry both meanings: escalate parks the run pending a colleague, reject terminates it. Collapsing them destroys the signal.

The terminal state, and why

Two durable artifacts, and what the second one does not do is the point.

The pause is rewritten with status="rejected" rather than cleared. Clearing it would make the run look like one that was never paused, and the reason it stopped is exactly what an auditor needs. Keeping it also makes the refusal enforceable: enforce_resume_authorization raises a new RunRejected — checked before expiry, because an operator told "the pause expired, re-approve it" would reasonably try to. No approval overrides it.

The run report gets canceled=True and a re-derived transaction_outcome. It does not force CANCELED. It states the human intent and lets classify_transaction_outcome decide what the evidence supports:

Rejection over… transaction_outcome
a halt whose consequential steps are positively proven effect-free CANCELED
a run that may already have written RECONCILIATION_REQUIRED

An operator tapping "stop" must never be able to convert an unreconciled write into a clean bill of health, and the absence-proof gate that prevents it is the same one every other absence-asserting outcome already passes through. No gate was weakened; canceled had no writer before this change, so nothing else is affected.

REJECTED_POLICY was considered and rejected: it means authorization/identity/qualification/environment refused execution, is derived from gate pseudo-steps, and conflating a machine policy gate with an operator's judgement would lose exactly the distinction this action exists to record.

execution_outcome is deliberately untouched. The run really did HALTED; transaction_outcome refines what is known about the business effect without rewriting the coarse lifecycle — the contract openadapt_flow/transaction.py documents.

Should reject be available on every halt?

Yes, except one — and the exception is not the halt category.

Gating on category was considered and is wrong here. Rejecting asserts something about this run — that it must not proceed — which an operator looking at the live application can conclude at a resolution halt as readily as at an effect halt. Worse, category gating would remove reject from exactly the halts where continue is offered (those with postconditions and a usable baseline) and leave it only where continue often is not. That inverts the reason it exists: the pressure toward the agreeable one-tap answer lives wherever the agreeable answer is one tap.

The one exception is a positively recorded uncertain delivery (pending.delivery_uncertainty). There the action may already have landed, the operator's task is reconciliation rather than termination, and escalate — which keeps the pause for someone who can reconcile — is the correct answer.

A trap worth recording: the gate must not read delivery_state == "unknown". That is _delivery_state's fail-closed default for most halts and means only that nothing proved non-delivery. An earlier revision of this branch keyed on it and withheld reject from nearly every pause — the whole feature, quietly disabled by a fail-closed default. Pinned by test.

What does the rejection assert?

That this run must not proceed. Not that the workflow is wrong — that claim changes future runs, and it belongs to teach, which carries the demonstration and requalification gate that authority requires. Reject changes nothing saved.

Does it require a reason?

A closed rejected_by_operator disposition, with exactly one member today, and no free text anywhere on the wire.

A reason taxonomy would make the distribution far more informative, and I deliberately did not invent one: there is no evidence yet for what its members should be — the reject rate is the data that would design them — and a wrong taxonomy is worse than none. Every extra required tap on the disagreement answer while continue stays one tap also re-creates the asymmetry the action exists to remove. The cause is an enum from the start precisely so adding members later is additive rather than a schema change, and so a free-text reason can never appear.

Behavioural verification

test_reject_terminates_the_run_and_the_run_report_says_so proves termination from report.json, not the UI: canceled: true, transaction_outcome: CANCELED, transaction_billable: false, execution_outcome still HALTED, and the pause retained as status="rejected". No live session is even provided, so nothing can be actuated on the way.

test_a_rejected_run_cannot_be_resumed_or_answered_again shuts both doors: a second attended answer (reject, escalate and continue) all 409, and an approved resume() raises RunRejected.

Mutation checktest_reject_admission_refuses_every_mutation_of_its_preconditions removes each precondition one at a time and asserts each refuses on its own: a disposition that does not belong to reject, no authenticated operator, a foreign capability digest, and a pause whose signed capability never carried reject. Each case also asserts the run was not ended.

test_reject_is_withheld_where_a_write_may_already_have_landed makes the delivery genuinely uncertain the production way — a continue whose executor never returns a terminal receipt — with no test backdoor, then asserts the phone route returns 409 naming the uncertainty and the correct next step.

Reverted during development, recorded so it is not re-attempted

Filtering reject out of the projected task once the journal shows an unresolved delivery. allowed_actions is inside the signed payload, so withdrawing an action mid-flight changes the task digest, and every request the phone already holds then fails as "the task changed" instead of with the specific refusal that tells the operator a write may have landed. It also made two existing tests fail for the right reason. execute_attended_action owns that refusal, exactly as it already does for continue and skip.

Local verification

tests/test_attended_actions.py + tests/test_mobile_attended_decision_e2e.py: 82 passed. ruff check openadapt_flow, ruff format --check openadapt_flow tests, and mypy (218 source files) all clean.

Downstream

openadapt-desktop (phone shell) and openadapt-cloud (dashboard) follow. Desktop's openadapt-flow== pin bumps to the release that carries this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM


Added on review: the RECONCILIATION_REQUIRED row was asserted, not proven

The table above claims a rejection over a run that may already have written
stays RECONCILIATION_REQUIRED. That is the whole safety argument for
re-deriving the outcome instead of forcing CANCELED — and no test covered
it. Every existing reject test ran over a halt where nothing had actuated, so
a correct re-derivation and a hardcoded CANCELED were indistinguishable.

test_rejecting_a_run_that_already_wrote_stays_reconciliation_required drives
a real run whose first step is an irreversible write that actuates and is
never verified, and only then halts for a human. The replayer produces the
report; no fixture is hand-edited. Rejecting is still accepted — refusing
to let an operator stop a run because it already wrote would be the wrong
failure — and transaction_outcome is RECONCILIATION_REQUIRED,
transaction_billable false, execution_outcome still HALTED, and the pause
rejected.

Mutation check. Replacing classify_transaction_outcome(report) with a
literal TransactionOutcome.CANCELED in _terminate_rejected_run fails this
test and no other — which is simultaneously the proof that the test is
load-bearing and that the property was previously unpinned.

This is distinct from the withheld case: reject is withheld only where the
runtime positively recorded that this step's delivery is uncertain. Here the
earlier step's delivery is certain and its effect is what is unknown, so the
operator can still legitimately end the run — they just cannot make the run
claim an absence.

tests/test_attended_actions.py + tests/test_mobile_attended_decision_e2e.py:
84 passed.

abrichr and others added 6 commits July 27, 2026 22:03
The attended vocabulary was closed at continue/skip/teach/escalate. When
the engine is CORRECT to halt -- the record genuinely says Ineligible --
the operator's only one-tap options were to assert they fixed something
or to hand the run to a colleague. There was no way to say "you were
right, stop". Disagreement therefore had no channel, so the recorded
answer distribution could not report how often a halt is correct, which
is the measurement needed before anyone tunes a halt rate.

`reject` is not a relabelled `escalate`. Escalate PARKS the run: the
durable pause stays intact and a qualified operator can still continue
it. Reject TERMINATES it. Merging them would be cheaper and would
destroy the signal the action exists to obtain.

Terminal semantics. The pause is rewritten with status="rejected" rather
than cleared, so the audit keeps WHY the run stopped, and
`enforce_resume_authorization` refuses with a distinct `RunRejected` that
no approval overrides. The run report gets canceled=True and a
re-derived `transaction_outcome`. It deliberately does NOT force
CANCELED: it states the human intent and lets the existing classifier
decide what the evidence supports, so a rejection over a run whose
consequential steps are proven effect-free is CANCELED while one over a
run that may have written stays RECONCILIATION_REQUIRED. An operator
tapping "stop" must never convert an unreconciled write into a clean
bill of health. `execution_outcome` is untouched: the run really did
halt, and the transaction outcome refines the business-effect claim
without rewriting the coarse lifecycle.

Gating. Reject is offered at every pause except one where the runtime
POSITIVELY recorded that the step may already have actuated
(`pending.delivery_uncertainty`), plus the matching live-journal check in
`execute_attended_action` for a delivery that became uncertain after the
capability was sealed. It is deliberately NOT gated on halt category:
rejecting asserts something about this run, which an operator can
conclude at a resolution halt as readily as at an effect halt, and
category gating would remove it from exactly the halts where `continue`
is offered. The gate does not read `delivery_state == "unknown"`, which
is the fail-closed default for most halts and would have withheld reject
almost everywhere.

The cause is a closed disposition with one member. A reason taxonomy
would be more informative, but there is no evidence yet for its members
-- the reject rate is the data that would design them -- and adding
members later is additive, whereas free text could never be closed
again.

openadapt-types floor raised to 0.6.3: `reject` and the rejected /
rejected_by_operator receipt pair are named symbols that only exist
there, and a resolver satisfied by 0.6.2 fails at import inside the
operator decision path.
The reviewed inventory pins the hash of every file that ships in a public
artifact. The attended console gained a reject button, so console.js
changed and the hash must be re-reviewed explicitly rather than drift.
The pause file is retained as the audit record of what was rejected, and
`attention_item` derived `durably_paused` from its mere presence -- so a
terminated run kept appearing in the queue as something awaiting an
answer, with an unknown status coerced to "pending". A terminated run
that still offers an answerable question is exactly the dangling pause
this action exists to avoid.

A rejected pause is now reported with status "rejected" and is not
durably paused, so no signed task is projected for it and the console
refuses every action before admission looks at the request. The engine's
own refusal, reachable by a relay still holding the pre-rejection
capability, is unchanged and still names the run as terminal.
An explicit decision, so it gets a test: losing the terminal transaction
outcome to a corrupt report.json must not lose the TERMINATION. The pause
status is the enforcement point, refusing to let an operator stop a run
because a JSON file will not parse would be the wrong failure to choose,
and the decision message says the outcome is unrecorded rather than
implying one was written.
The rejection terminator loaded report.json through
`openadapt_flow.console.data`, which made `runtime/durable` the only
module in the runtime importing the console. The console is an optional
extra and a presentation layer; the runtime must not acquire a dependency
on it in order to end a run. Reads the report directly instead. No import
weight either way -- console.data is already import-light -- but the
layering was backwards.
…aim absence

The branch's central safety claim -- an operator tapping "stop" must
never convert an unreconciled write into a clean bill of health -- had no
test. `_terminate_rejected_run` sets `canceled=True` and RE-DERIVES the
transaction outcome rather than forcing CANCELED, precisely so the
absence-proof gate can still dominate, but every existing reject test
ran over a halt where nothing had actuated, so CANCELED and a forced
CANCELED were indistinguishable.

This drives a real run whose FIRST step is an irreversible write that
actuates and is never verified, and only then halts for a human. No
fixture hand-editing: the replayer produces the report. Rejecting is
still ACCEPTED -- refusing to let an operator stop a run because it
already wrote would be the wrong failure -- but the terminal
`transaction_outcome` stays RECONCILIATION_REQUIRED.

Mutation-checked: replacing `classify_transaction_outcome(report)` with
a literal `TransactionOutcome.CANCELED` fails this test and NO other,
which is also the proof the property was previously unpinned.

Note this is distinct from the withheld case. Reject is withheld only
where the runtime positively recorded that THIS step's delivery is
uncertain; here delivery of the earlier step is certain and its EFFECT
is what is unknown, so the operator can still legitimately end the run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM
@abrichr
abrichr force-pushed the feat/attended-reject-action branch from 05c6bed to b343277 Compare July 28, 2026 02:07
@abrichr
abrichr enabled auto-merge (squash) July 28, 2026 02:10
@abrichr
abrichr merged commit 736ff8f into main Jul 28, 2026
17 checks passed
@abrichr
abrichr deleted the feat/attended-reject-action branch July 28, 2026 02:39
abrichr added a commit that referenced this pull request Jul 28, 2026
Two defects, found when `reject` (#295) landed on main and this branch was
brought up to date.

`execute` hardcoded `disposition="completed_by_operator"`.
`execute_attended_action` refuses a mismatched (action, disposition) pair by
design -- `skip` wants `not_applicable`, `teach` wants `teach_requested`,
`escalate` wants `cannot_complete`, `reject` wants `rejected_by_operator`. So
FOUR of the five answers a phone can give would have been refused AFTER the
operator gave them, with a message naming a disposition they never chose. Only
Continue worked, and nothing said so.

`_ENGINE_ACTIONS` was a hand-written copy of the vocabulary, frozen at four
members before `reject` existed. The control plane already emits `reject` as a
`FlowAttendedAction`, so a phone offering it would have had the answer rejected
by the relay's own shape check.

Both now come from one `_ENGINE_DISPOSITION` map, and a test asserts the
relayed vocabulary EQUALS `AttendedActionRequest`'s, so the next action added to
the engine fails this test instead of silently being undeliverable to a phone.
`reject` terminates the run and `escalate` parks it; they are separate members
for that reason, and a phone that offers Reject must be able to deliver it or
the answer distribution loses the disagreement signal the action exists to
record.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM
abrichr added a commit that referenced this pull request Jul 28, 2026
PR #295 made `reject` an always-available attended action, so the engine
now emits it in `action_vocabulary` and in `allowed_actions` for every
halt category. The committed artifact predates that change and still
lists four actions.

Regenerated with OPENADAPT_UPDATE_GOLDEN=1. The vocabulary assertion
compares `_ACTION_MAP` against `openadapt_types.HumanDecisionAction`
rather than a hand-copied list, which is what caught the drift.
abrichr added a commit that referenced this pull request Jul 28, 2026
PR #295 made `reject` an always-available attended action, so the engine
now emits it in `action_vocabulary` and in `allowed_actions` for every
halt category. The committed artifact predates that change and still
lists four actions.

Regenerated with OPENADAPT_UPDATE_GOLDEN=1. The vocabulary assertion
compares `_ACTION_MAP` against `openadapt_types.HumanDecisionAction`
rather than a hand-copied list, which is what caught the drift.
abrichr added a commit that referenced this pull request Jul 28, 2026
#296)

* feat(decisions): deliver halts to a phone without customer TLS ingress

An attended halt could reach a phone one of two ways, and neither worked for
a customer without an IT department:

* the runner-local portal carries the whole local `presentation` -- screen
  crops, the gated control label, the halt detail -- but `customer_ingress`
  mode requires an https origin the customer terminates themselves; and
* the hosted lane is reachable from anywhere, but `portable_remote_decision_task`
  discarded `presentation` wholesale, and -- more to the point -- NOTHING ON A
  RUNNER EVER CALLED IT. The endpoints, the relay table and the dashboard exist
  in the control plane; there is no client for them in this repo, in Desktop,
  or anywhere else in the workspace.

So the path that could carry the context was unreachable, and the reachable
path was not wired at all.

This closes both halves.

The context. `halt_detail` already produces closed enums, bounded integers and
booleans, with exactly one exception: `target_label`, the target control's own
accessible name, which `_safe_target_label` releases only after six independent
proofs. That single string is why the whole block stayed local.
`decision_context.RemoteHaltContextV1` re-derives the block WITHOUT it, so it
has no string-valued field at all -- structurally incapable of representing a
patient name, an MRN, an observed value, a path or a workflow label, in the
same sense `HumanDecisionTaskV1` already is. The phone says "could not find the
button" instead of "the button labelled 'Open'". That is the whole cost.

The ladder. `DecisionDeliveryTier` grades delivery the way `VerificationTier`
grades evidence: local_full > remote_closed_context > remote_identifiers >
notification_only. A deployment's `context_tier` and the execution profile's
`max_remote_decision_tier` both apply and the weaker wins; `local_full` is
refused as a remote tier by name. A projection records the tier it ACTUALLY
delivered, so an engine that could not build a context reports the lower one
rather than claiming the higher.

The transport. `decision_relay.DecisionRelay` dials OUT -- no inbound port, no
port forward, no certificate, no static address, works behind NAT -- following
the register/poll/lease/ack shape Desktop's runner lane already uses. A relayed
answer is presentation and authentication evidence, never execution authority:
it is admitted through the unchanged `execute_remote_attended_action`, so the
exact pause capability, the single-flight lease, the fresh live re-read and
every `will_recheck` contract still gate the continuation.

Uncertainty is classified, never guessed. A POST that leaves the process
without a terminal response returns `unknown`, is not retried, and is not
described as reachable. The word "delivered" appears nowhere in the relay: a
successful POST proves the control plane accepted a task, not that a person
received or read it.

Two things worth reviewing closely:

* `delivery_tier` and `halt_context` are deliberately NOT in `binding_digest`.
  `admit_remote_action` re-derives the binding on every response including a
  replayed one, so it must be a deterministic function of the signed
  capability. The halt context is read from the run's live checkpoint, which
  legitimately empties once the pause closes. Binding it made a correct
  idempotent replay refuse -- caught by the existing
  `test_remote_decision_is_idempotent_and_resumes_through_fresh_remote_actuation`,
  and now pinned directly by
  `test_a_replayed_decision_still_admits_after_the_pause_closed`.
* `test_the_verifier_agrees_with_the_hosted_signer_byte_for_byte` pins the
  relay HMAC against a fixture produced by running the control plane's own
  `signHumanDecisionRelay`/`canonicalJson` in Node. Neither side's
  canonicalization can drift without that test failing.

The signed `HumanDecisionTaskV1` is byte-identical at every tier;
`test_the_signed_task_is_byte_identical_at_both_remote_tiers` asserts it.

`docs/DECISION_DELIVERY.md` records the ladder, exactly what a customer has to
do (no TLS), why PHI-scrubbing the presentation was rejected as a delivery path
on the evidence rather than on principle, why LAN-only and notification-only
are not the answer, and the E2EE blind-relay design for the screen frame --
written down rather than half-implemented.

The hosted side must accept the two new projection fields before this lane
carries context end to end; that is a separate PR against the control plane.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM

* fix(decisions): bind the delivery ceiling to the run's actual profile

`_remote_delivery_tier` read only `deployment.runtime.profile`. A governed
dispatch carries its own execution profile, which the local deployment file
need not name -- a hosted runner is handed its authorization at dispatch time.
So a run executed as `regulated` could be projected under a `demo` ceiling.

It now consults `RunReport.execution_profile` as well and takes the weakest of
the three ceilings (deployment `context_tier`, deployment profile, run
profile). A report with no profile takes `regulated`, matching
`resolve_execution_profile`'s default.

The doc is also corrected to say plainly that `max_remote_decision_tier` is not
yet discriminating -- every profile currently permits `remote_closed_context`,
because that tier widens what an operator KNOWS without widening what the
envelope can REPRESENT. It becomes load-bearing when a tier exists that a
profile must refuse.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM

* docs(exception-inbox): record the new delivery tier and the outbound relay

The remote-projection description said the projection carries only the
closed-enum task and its bindings, and that Flow supplies no remote network
provider. Both were true before this branch and are not now: the projection may
carry the closed halt context, and `decision_relay.DecisionRelay` is an
outbound-only network client. Flow still supplies no remote AUTHENTICATION
provider.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM

* fix(decisions): type the halt-context schema constant as its literal

`mypy` (the default-config run in the lint gate) rejected assigning the
`str` constant to the model's `Literal` field. Annotate the constant with the
literal type so the single source of the schema string keeps type-checking,
rather than duplicating the string or silencing the check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM

* test(halt-detail): stop a digest collision reading as a PHI leak

`test_a_typed_value_degrades_to_the_field_shape_and_never_appears` asserts that
`40182` -- the MRN inside this file's protected value -- is absent from the
serialized projection. It failed in CI on a digest:

    sha256:...06e07a769c4018288e5ae44790cfc9e0471d0757b0ee...
                       ^^^^^

A sha256 digest is 64 hexadecimal characters, and hex includes every decimal
digit, so a 5-digit identifier lands inside one by chance roughly once every few
thousand runs. It reads as a PHI leak and is not one: a one-way digest cannot
carry content, and the projection was correct.

The scan now replaces every digest, signature and opaque id with a marker before
serializing, so it covers exactly the fields where a leak could live. This
narrows nothing that was holding: the removed values' exact shapes are already
pinned field by field by
`test_the_local_detail_carries_only_closed_values`, and a new test asserts the
helper strips the colliding digest while leaving a content-bearing field alone,
so a real leak still fails.

Pre-existing on origin/main (the assertion arrived in #290); it surfaced here
because this branch's CI run drew the collision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM

* fix(decisions): let the relay carry every answer, not only Continue

Two defects, found when `reject` (#295) landed on main and this branch was
brought up to date.

`execute` hardcoded `disposition="completed_by_operator"`.
`execute_attended_action` refuses a mismatched (action, disposition) pair by
design -- `skip` wants `not_applicable`, `teach` wants `teach_requested`,
`escalate` wants `cannot_complete`, `reject` wants `rejected_by_operator`. So
FOUR of the five answers a phone can give would have been refused AFTER the
operator gave them, with a message naming a disposition they never chose. Only
Continue worked, and nothing said so.

`_ENGINE_ACTIONS` was a hand-written copy of the vocabulary, frozen at four
members before `reject` existed. The control plane already emits `reject` as a
`FlowAttendedAction`, so a phone offering it would have had the answer rejected
by the relay's own shape check.

Both now come from one `_ENGINE_DISPOSITION` map, and a test asserts the
relayed vocabulary EQUALS `AttendedActionRequest`'s, so the next action added to
the engine fails this test instead of silently being undeliverable to a phone.
`reject` terminates the run and `escalate` parks it; they are separate members
for that reason, and a phone that offers Reject must be able to deliver it or
the answer distribution loses the disagreement signal the action exists to
record.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM

---------

Co-authored-by: Claude Opus 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