From e7df084236d2255b149128050bf78c651e0f6231 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Mon, 27 Jul 2026 23:22:26 -0400 Subject: [PATCH 1/2] feat(attended): record WHAT KIND of decider answered, not only which identity An agreement rate drawn from the attended journal is a claim about what PEOPLE concluded when they read the evidence. The journal could not support that claim, and nothing said so. `AttendedDecision.operator` records WHICH identity a decision is attributed to. It has never recorded WHAT KIND of decider produced it, and the two are different questions. The MCP bridge in `openadapt-agent` exposes `continue_attention` and `skip_attention` as model-callable tools, and derives its operator from the same `_local_operator_identity()` a person's own console uses. So a decision a MODEL submitted and one a PERSON submitted from the same machine carried an IDENTICAL operator string. Every rate computed over this journal mixed the two populations with no way to separate them afterwards. `decided_by` is a closed enum -- human / automation / unknown -- beside `operator`, asserted by the trusted caller in exactly the same way. It is deliberately NOT a field on `AttendedActionRequest`. That is what a relay or an MCP client puts on the wire and `request_digest` commits to it, so putting provenance there would let the party whose nature is in question be the party declaring it. Pinned by test. The default is `unknown`, not `human`, and that is the safety property. A caller that does not declare is EXCLUDED from a human rate rather than silently counted into it, and every decision written before this field existed reads back as `unknown` -- which is true, because the bridge predates the field. Defaulting to `human` would have re-created the same bias one level down, where nobody would look for it. The console and the authenticated AAL2 relay therefore assert `human` explicitly, and a test pins that they do: if they ever stopped, genuine operator answers would fall silently out of the sample and the failure would look like nothing. `AttendedActionService.execute` relays the value unchanged. It owns the live executor, not the question of who decided, and inventing an answer there would attribute a decision on behalf of a caller that never made the claim. No free text is added anywhere, and the PHI-free `HumanDecisionReceiptV1` is untouched: it carries no operator identity by design, and provenance belongs to the local append-only audit record that already carries one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM --- openadapt_flow/console/app.py | 7 + openadapt_flow/console/human_decisions.py | 4 + openadapt_flow/runtime/durable/attended.py | 44 ++++- .../runtime/durable/attended_service.py | 10 +- tests/test_attended_actions.py | 3 +- tests/test_mobile_attended_decision_e2e.py | 168 +++++++++++++++++- 6 files changed, 231 insertions(+), 5 deletions(-) diff --git a/openadapt_flow/console/app.py b/openadapt_flow/console/app.py index 04d6ba1b..fd86c0cf 100644 --- a/openadapt_flow/console/app.py +++ b/openadapt_flow/console/app.py @@ -607,17 +607,24 @@ def execute_attention_action( "the attention item is no longer current; reload the queue" ) request = human_decisions.admit_console_action(run_dir, item, payload) + # A person answered on the console or the paired phone: the + # operator identity is server-derived from the local OS session and + # the request arrived through a browser. Asserted explicitly rather + # than left to default, because `unknown` is reserved for a caller + # that genuinely did not say. decision = ( attended_service.execute( run_dir, request, operator=operator, + decided_by="human", ) if attended_service is not None else execute_attended_action( run_dir, request, operator=operator, + decided_by="human", ) ) except (AttendedActionRefused, ResumeRefused) as exc: diff --git a/openadapt_flow/console/human_decisions.py b/openadapt_flow/console/human_decisions.py index d49057dc..265e2767 100644 --- a/openadapt_flow/console/human_decisions.py +++ b/openadapt_flow/console/human_decisions.py @@ -640,10 +640,14 @@ def execute_remote_attended_action( deployment=deployment, principal=principal, ) + # A remote person answered. The relay admits a decision only against an + # authenticated principal whose assurance the control plane raised to AAL2, + # so this path cannot carry a model's answer. return execute_attended_action( run_dir, engine_request, operator=principal.subject, + decided_by="human", executor=executor, key=key, ) diff --git a/openadapt_flow/runtime/durable/attended.py b/openadapt_flow/runtime/durable/attended.py index b4408cfb..ef567aad 100644 --- a/openadapt_flow/runtime/durable/attended.py +++ b/openadapt_flow/runtime/durable/attended.py @@ -241,6 +241,36 @@ class AttendedDecision(BaseModel): idempotency_key: str action: Literal["continue", "skip", "reject", "teach", "escalate"] operator: str + #: WHAT KIND of decider produced this answer, as distinct from ``operator``, + #: which records WHICH IDENTITY it was attributed to. These are different + #: questions and only one of them was answerable before this field. + #: + #: ``operator`` is the local OS account or the authenticated remote + #: subject. The MCP bridge in ``openadapt-agent`` derives it from the same + #: ``_local_operator_identity()`` a person's own console does, so a + #: decision a MODEL submitted and one a PERSON submitted from the same + #: machine carried an IDENTICAL ``operator`` string. Any rate computed over + #: this journal therefore mixed the two populations with no way to separate + #: them afterwards. + #: + #: That matters most for the exact measurement the attended vocabulary + #: exists to produce. An agreement rate is a claim about what PEOPLE + #: concluded when they read the evidence. A model's answers are a different + #: population, and averaging them together answers neither question. + #: + #: Caller-asserted, exactly like ``operator``, and deliberately NOT a field + #: on :class:`AttendedActionRequest`: the request is what a relay or an MCP + #: client sends and ``request_digest`` commits to it, so a submitter must + #: never be able to describe its own nature. The trusted caller that + #: already asserts ``operator`` asserts this beside it. + #: + #: The default is ``unknown``, and that is deliberate. A caller that does + #: not declare is EXCLUDED from a human rate rather than silently counted + #: into it, and every decision written before this field existed reads back + #: as ``unknown`` -- which is the truth, because the bridge predates it. + #: Defaulting to ``human`` would have re-created the same bias one level + #: down. A closed enum, never free text; a new member is additive. + decided_by: Literal["human", "automation", "unknown"] = "unknown" disposition: Optional[str] = None status: Literal[ "prepared", @@ -1171,11 +1201,18 @@ def execute_attended_action( request: AttendedActionRequest, *, operator: str, + decided_by: Literal["human", "automation", "unknown"] = "unknown", executor: Optional[AttendedActionExecutor] = None, key: Optional[str] = None, now: Optional[datetime] = None, ) -> AttendedDecision: - """Admit and execute one attended decision under exact binding.""" + """Admit and execute one attended decision under exact binding. + + ``decided_by`` records WHAT KIND of decider this is, beside ``operator``, + which records WHICH identity. See :class:`AttendedDecision` for why the two + are separate questions and why the default is ``unknown`` rather than + ``human``. + """ from openadapt_flow import crypto as _crypto key = _crypto.resolve_key(key) @@ -1269,6 +1306,7 @@ def execute_attended_action( idempotency_key=request.idempotency_key, action=request.action, operator=operator, + decided_by=decided_by, disposition=request.disposition or "rejected_by_operator", status="rejected", message=( @@ -1293,6 +1331,7 @@ def execute_attended_action( idempotency_key=request.idempotency_key, action=request.action, operator=operator, + decided_by=decided_by, disposition=request.disposition or "teach_requested", status="needs_demonstration", message=( @@ -1314,6 +1353,7 @@ def execute_attended_action( idempotency_key=request.idempotency_key, action=request.action, operator=operator, + decided_by=decided_by, disposition=request.disposition or "needs_assistance", status="escalated", message=( @@ -1346,6 +1386,7 @@ def execute_attended_action( idempotency_key=request.idempotency_key, action=request.action, operator=operator, + decided_by=decided_by, disposition=request.disposition, status="prepared", message="request admitted; no delivery attempted", @@ -1392,6 +1433,7 @@ def execute_attended_action( idempotency_key=request.idempotency_key, action=request.action, operator=operator, + decided_by=decided_by, disposition=request.disposition, status=result.status, message=result.message, diff --git a/openadapt_flow/runtime/durable/attended_service.py b/openadapt_flow/runtime/durable/attended_service.py index fe0ceba9..8cb0d865 100644 --- a/openadapt_flow/runtime/durable/attended_service.py +++ b/openadapt_flow/runtime/durable/attended_service.py @@ -353,14 +353,22 @@ def execute( request: AttendedActionRequest, *, operator: str, + decided_by: Literal["human", "automation", "unknown"] = "unknown", ) -> AttendedDecision: - """Admit and execute one exact attended decision on the live session.""" + """Admit and execute one exact attended decision on the live session. + + ``decided_by`` is relayed unchanged. This service owns the live + executor, not the question of who decided, and inventing an answer here + would attribute a decision on behalf of a caller that never made the + claim. + """ if not self._entered: raise RuntimeError("attended action service is not open") return execute_attended_action( run_dir, request, operator=operator, + decided_by=decided_by, executor=self._owner, key=self._key, ) diff --git a/tests/test_attended_actions.py b/tests/test_attended_actions.py index ff6a7517..48e20270 100644 --- a/tests/test_attended_actions.py +++ b/tests/test_attended_actions.py @@ -1882,11 +1882,12 @@ def test_attended_http_action_requires_auth_csrf_and_exact_capability( executor = _ResultExecutor() class Service: - def execute(self, run_dir, request, *, operator): + def execute(self, run_dir, request, *, operator, decided_by="unknown"): return execute_attended_action( run_dir, request, operator=operator, + decided_by=decided_by, executor=executor, ) diff --git a/tests/test_mobile_attended_decision_e2e.py b/tests/test_mobile_attended_decision_e2e.py index 8debc4d6..add1d23c 100644 --- a/tests/test_mobile_attended_decision_e2e.py +++ b/tests/test_mobile_attended_decision_e2e.py @@ -138,11 +138,20 @@ class _Service: def __init__(self, executor): self.executor = executor self.calls = 0 + self.deciders = [] - def execute(self, run_dir, request, *, operator): + def execute(self, run_dir, request, *, operator, decided_by="unknown"): self.calls += 1 + # Recorded, not defaulted: the console must state that a person + # decided, and a fake that quietly accepted `unknown` would hide a + # console that stopped saying so. + self.deciders.append(decided_by) return execute_attended_action( - run_dir, request, operator=operator, executor=self.executor + run_dir, + request, + operator=operator, + decided_by=decided_by, + executor=self.executor, ) @@ -1471,3 +1480,158 @@ def test_rejecting_a_run_that_already_wrote_stays_reconciliation_required( ), operator="front-desk", ) + + +# --------------------------------------------------------------------------- +# Who decided: separating a person's answer from a model's +# --------------------------------------------------------------------------- + + +def _journal(run): + """Every decision the engine wrote for this run, oldest first.""" + return AttendedActionStore(run)._read_log().decisions + + +def test_a_human_answer_and_an_automated_answer_are_told_apart_in_the_journal( + tmp_path, monkeypatch +): + """An agreement rate must be computable over PEOPLE alone. + + ``operator`` could never support that. It is the local OS account, and the + MCP bridge in ``openadapt-agent`` derives it from the same + ``_local_operator_identity()`` the console does -- so a model's answer and a + person's answer from one machine carried an IDENTICAL operator string. The + two populations were mixed with no way to separate them afterwards, which + makes every rate drawn from this journal a claim nobody can qualify. + + This drives both answers into one journal with the SAME operator, and then + does the thing an analyst would actually do: filter to human decisions. If + the filter cannot exclude the model's answer, the field is decoration. + """ + from openadapt_flow.runtime.durable.attended import AttendedActionRequest + + _wf, bundles, runs, _bundle, run, capability = _halt(tmp_path, name="mixed") + _app, client = _phone(bundles, runs, monkeypatch) + item, detail = _open_task(client) + + # (1) A person answers through the phone route. + human = _post( + client, item, _decision(detail, action="escalate", key="human-escalate-0001") + ) + assert human.status_code == 200 + + # (2) An automated caller answers the same pause, asserting its own nature + # the way the MCP bridge does -- and deliberately claiming the SAME + # operator identity, because that is exactly what it does today. + execute_attended_action( + run, + AttendedActionRequest( + capability_digest=capability.digest, + idempotency_key="automation-escalate-001", + action="escalate", + disposition="needs_assistance", + ), + operator="front-desk", + decided_by="automation", + ) + + decisions = _journal(run) + assert len(decisions) == 2 + + # The identity field cannot tell them apart. This is the defect, pinned so + # nobody later "simplifies" the new field away by pointing at this one. + assert len({d.operator for d in decisions}) == 1, ( + "the premise of this test is that operator is identical for both" + ) + + # The provenance field can. + assert {d.decided_by for d in decisions} == {"human", "automation"} + + # -- the measurement consequence, stated as the query itself -------------- + human_only = [d for d in decisions if d.decided_by == "human"] + assert len(human_only) == 1 + assert all(d.decided_by == "human" for d in human_only) + # And the automated answer is genuinely excluded, not merely labelled. + assert "automation-escalate-001" not in {d.idempotency_key for d in human_only} + + +def test_an_undeclared_caller_is_excluded_from_a_human_rate_not_counted_into_it( + tmp_path, +): + """The default is the whole safety property of this field. + + Had it defaulted to ``human``, every caller that forgot to declare -- and + every decision written before the field existed -- would be counted as a + person, which re-creates the same bias one level down and is invisible. + ``unknown`` is the honest value: we do not know, so the record says so and + a human rate leaves it out. + """ + from openadapt_flow.runtime.durable.attended import AttendedActionRequest + + _wf, _bundles, _runs, _bundle, run, capability = _halt(tmp_path, name="silent") + + execute_attended_action( + run, + AttendedActionRequest( + capability_digest=capability.digest, + idempotency_key="undeclared-escalate-01", + action="escalate", + disposition="needs_assistance", + ), + operator="front-desk", + ) + + decision = _journal(run)[-1] + assert decision.decided_by == "unknown", ( + "an undeclared caller must never be silently attributed to a person" + ) + assert [d for d in _journal(run) if d.decided_by == "human"] == [] + + +def test_the_console_and_the_phone_route_assert_a_person_decided(tmp_path, monkeypatch): + """The default is ``unknown``, so the human paths must SAY they are human. + + This is the half of the contract the default cannot provide. If the console + ever stopped asserting it, every genuine operator answer would silently + fall out of the human population and the rate would quietly measure a + shrinking sample -- a failure that looks like nothing at all. + + Both branches are covered: the live-executor branch through the injected + service, and the direct branch through the journal. + """ + _wf, bundles, runs, _bundle, run, _capability = _halt(tmp_path, name="asserts") + backend, executor = _completed_live_session() + service = _Service(executor) + _app, client = _phone(bundles, runs, monkeypatch, service=service) + item, detail = _open_task(client) + + assert ( + _post( + client, item, _decision(detail, action="continue", key="console-human-0001") + ).status_code + == 200 + ) + assert service.deciders == ["human"], service.deciders + assert _journal(run)[-1].decided_by == "human" + assert backend.actions, "the live branch was not the one exercised" + + +def test_a_submitter_cannot_describe_its_own_nature(tmp_path): + """``decided_by`` is asserted by the trusted caller, never sent in the request. + + ``AttendedActionRequest`` is what a relay or an MCP client puts on the + wire, and ``request_digest`` commits to it. If provenance lived there, the + party whose nature is in question would be the party declaring it. It sits + beside ``operator`` instead, which is asserted the same way and for the + same reason. + """ + from openadapt_flow.runtime.durable.attended import AttendedActionRequest + + with pytest.raises(ValidationError): + AttendedActionRequest( + capability_digest="sha256:" + "0" * 64, + idempotency_key="forged-provenance-0001", + action="escalate", + disposition="needs_assistance", + decided_by="human", + ) From e389c0d55a7bad67c9a0ee8e75aa2cb47aeb7381 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 28 Jul 2026 01:23:40 -0400 Subject: [PATCH 2/2] test(attended): update service contract fixtures --- tests/test_console.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/test_console.py b/tests/test_console.py index 609ff8dc..110ecc37 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -1768,10 +1768,12 @@ def skip_run(self, _run_dir, _capability, _approval): ) monkeypatch.setattr( "openadapt_flow.runtime.durable.attended_service.execute_attended_action", - lambda _run_dir, _request, *, operator, executor, key: executor.continue_run( - Path("run"), - SimpleNamespace(step_id=f"step-for-{operator}"), - SimpleNamespace(), + lambda _run_dir, _request, *, operator, executor, key, decided_by="unknown": ( + executor.continue_run( + Path("run"), + SimpleNamespace(step_id=f"step-for-{operator}"), + SimpleNamespace(), + ) ), ) @@ -1927,10 +1929,12 @@ def fake_configured(_backend, **_kwargs): ) monkeypatch.setattr( "openadapt_flow.runtime.durable.attended_service.execute_attended_action", - lambda _run_dir, _request, *, operator, executor, key: executor.continue_run( - Path("run"), - SimpleNamespace(step_id=f"step-for-{operator}"), - SimpleNamespace(), + lambda _run_dir, _request, *, operator, executor, key, decided_by="unknown": ( + executor.continue_run( + Path("run"), + SimpleNamespace(step_id=f"step-for-{operator}"), + SimpleNamespace(), + ) ), ) @@ -2260,8 +2264,11 @@ def test_public_attended_action_service_contract_is_stable(): "run_dir", "request", "operator", + "decided_by", ] assert execute.parameters["operator"].kind is inspect.Parameter.KEYWORD_ONLY + assert execute.parameters["decided_by"].kind is inspect.Parameter.KEYWORD_ONLY + assert execute.parameters["decided_by"].default == "unknown" service = AttendedActionService(DeploymentConfig()) assert not hasattr(service, "continue_run")