feat(effects): exact-new-set guard closes the over-write false-pass gap - #362
Merged
Conversation
`_judge_record_written` verifies that the DECLARED record matches at the expected cardinality and, via `forbid_collateral_loss`, that no pre-existing record vanished. Nothing verified that the action added no records BEYOND the declared set. An actuation that wrote the intended record AND N unintended ones therefore satisfied every declared contract and was CONFIRMED: a FALSE PASS in the shipping runtime, the one error direction this design must never take. Measured, not hypothetical -- in a 150-trial benchmark study an agent asked to download 6 records downloaded 37, and every declared-record contract confirmed. Add EffectKind.EXACT_NEW_SET: one table-scoped effect declaring the FULL set of records an action may add (`new_records`, `expected_count == len(new_records)`, `identity_field`, `match` as SCOPE). It refutes an addition no member names, a missing or duplicated member, a wrong cardinality, and collateral loss inside the scope. It requires a REAL pre-action baseline and returns a structured INDETERMINATE refusal -- never a guess -- when the baseline is unreachable or a record carries no `identity_field` value. It rides flow's existing machinery unchanged: contract loading, the shared judge, aggregation, coverage, evidence tiers, receipts, the effect journal, and `EffectVerdict`. `Effect.requires_baseline` replaces the open-coded `count_new_only or forbid_collateral_loss` pre-state test in the candidate selector and the replayer, and the current-state read-back paths (durable resume, attended qualified read-back) now refuse a delta claim instead of judging it against a synthesized empty baseline. Backward compatibility: the kind is additive and OPT-IN. Flow contracts are operator-authored -- there is no derivation step that could default it on -- so every existing contract judges exactly as before and keeps a byte-identical `contract_hash` (the new fields enter the digest only on the new kind), pinned by a regression test. The honest boundary is stated in docs/EFFECT_KIT.md: an existing contract does not detect an over-write unless the operator declares this effect. The three shared oracle modules are ported so they stay a VERBATIM extraction of openadapt-attest's `openadapt_attest/oracle/` (attest main 3056285), modulo the import rewrites its UPSTREAM.lock declares, keeping the next re-extraction clean and its drift job meaningful. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ct fields `_workflow_content()` dumps the whole workflow into the sealed content digest, so `Effect.new_records` and `Effect.identity_field` entered every bundle's digest the moment they existed. Any bundle sealed BEFORE this change recomputed to a different digest and failed integrity verification with `BundleIntegrityError`. That is a backward-compatibility break for every customer holding a sealed bundle whose steps declare an effect contract, not merely a fixture that needs regenerating -- so the committed fixture is left untouched. Add the two reviewed, version-scoped `_SealedCanonicalOmission` entries the registry's own comment prescribes, owned by `Effect` and scoped to schema version 2, each omitted only at its semantically-empty default (`[]` and `"id"`). They follow the existing entries' shape exactly: exact model ownership, explicit omitted values, and a reason. No recursive key-name removal and no global `exclude_defaults`. Regressions, in `tests/test_bundle_schema_v2.py`: - `test_default_exact_new_set_fields_preserve_pre_field_sealed_v2_digest` seals a synthetic bundle whose step carries a `record_written` contract and loads it with integrity verification. Its pinned digest (89f3648282f337a0...) was computed by the engine as it stood BEFORE these fields existed, so the test cannot merely agree with the new code. - `test_a_declared_exact_new_set_is_sealed_content` proves the omission does not weaken the digest for a contract that USES the guard: a different declared set, an extra declared member, and a non-default identity column each change the digest. - The cross-version test now also asserts both fields survive at schema version 3, so the rules never carry into a future schema implicitly. - The pre-field rendering helpers navigate to the exact owning `Effect` objects rather than stripping a key name anywhere it appears. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap this closes
openadapt_flow/runtime/effects/_common.py::_judge_record_writtenverifies twothings: that the declared record matches at the expected cardinality, and —
via
forbid_collateral_loss— that no pre-existing record vanished.Nothing verified that the action added no records beyond the declared set.
An actuation that wrote the intended record and N unintended ones therefore
satisfied every declared contract and came back CONFIRMED. That is a false
pass in the shipping runtime — the one error direction this design must never
take. Every other kind answers "is my record there?"; none of them answered
"and nothing else?".
This is measured, not hypothetical. In a 150-trial benchmark study, an agent
asked to download 6 records downloaded 37. All 6 declared rows existed, so
every per-record contract confirmed while the system of record held 31 writes
nobody asked for.
The fix
EffectKind.EXACT_NEW_SET— one table-scoped effect declaring the FULL setof records an action may add:
It REFUTES an addition no member names (the guard), a missing or duplicated
member, a wrong cardinality, and collateral loss inside the same scope. It
requires a real pre-action baseline and issues a structured
INDETERMINATE refusal — never a guess — when the baseline is unreachable or
a record on either side carries no
identity_fieldvalue.new_records: []with
expected_count: 0is the meaningful assertion "this action adds NOTHINGto this read set."
It rides flow's existing machinery unchanged: contract loading, the shared
judge, aggregation, coverage, evidence tiers, receipts, the effect journal, and
EffectVerdict.Effect.requires_baselinereplaces the open-codedcount_new_only or forbid_collateral_losspre-state test in the candidateselector and the replayer, and the current-state read-back paths (durable
resume, attended qualified read-back) now refuse a delta claim rather than judge
it against a synthesized empty baseline.
Existing contracts are unchanged; opt-in is required
The kind is additive and opt-in. Flow contracts are operator-authored —
unlike
openadapt-attest'sderive, flow has no derivation step that coulddefault the guard on. So:
contract_hashis byte-identical (the new fields enter thedigest only on the new kind) — pinned by a regression test against digests
computed at the parent commit;
docs/EFFECT_KIT.md: an existingcontract does not detect an over-write unless the operator declares an
exact_new_seteffect for that read set.Restored verbatim relationship with openadapt-attest
openadapt-attest's oracle tree is a verbatim extraction ofopenadapt_flow/runtime/effects/(itsUPSTREAM.lock+scripts/check_upstream_drift.py). The guard landed there first(
OpenAdaptAI/openadapt-attestmain3056285, PR #10), so this port restoresthe extraction: after this change all 16 shared modules — the whole
runtime/effects/package plusverification.py— are byte-for-byteidentical to attest's copies modulo the
import_rewritesitsUPSTREAM.lockdeclares. attest's
UPSTREAM.lockneeds a deliberate re-extraction pass afterthis merges (its drift job will name
_common.py,effect.py,adapter.py),which is exactly what that job is for.
Sealed-bundle compatibility (second commit)
_workflow_content()feedsworkflow.model_dump(...)into the sealed contentdigest, so the two additive
Effectfields entered every bundle's digest themoment they existed. Any bundle sealed before this change recomputed to a
different digest and failed with
BundleIntegrityError— a realbackward-compatibility break for any customer holding a sealed bundle whose
steps declare an effect contract. CI caught it as
test_committed_encounter_showcase_loop_replays_through_the_interpreter. Thecommitted fixture was not regenerated: that would have hidden the break.
The fix is the one the registry's own comment prescribes — two reviewed,
version-scoped
_SealedCanonicalOmissionentries owned byEffect, scoped toschema version 2, omitted only at their semantically-empty defaults (
[]and"id"), following the existing entries' shape exactly. No recursive key-nameremoval, no global
exclude_defaults.Historical seal regressions (
tests/test_bundle_schema_v2.py):record_writtencontract, sealedpre-field, still validates. Its pinned digest
89f3648282f337a0…wascomputed by the engine as it stood before these fields existed, so the
regression cannot merely agree with the new code;
extra declared member, and a non-default identity column each change the
digest;
implicitly.
The rules are value-scoped like every other registry entry, so an
exact_new_seteffect that keeps the defaultidentity_field: idhas that keyomitted. That does not weaken the contract:
kind,expected_countand thedeclared set are all sealed, and any other identity column is sealed too.
Audited, unchanged by design:
qualification.workflow_contract_sha256()does shift for a workflow that declares an effect
(
fa70cbc7…→be5a923c…, measured). It applies no canonical omissions andnever has — PR #291 (
selection_region) set the same precedent and likewisedid not touch it. It is a within-generation contract identity that is meant
to change when executable intent changes, and it fails closed (a
BINDING_MISMATCHfinding requiring re-review), never open. Retrofitting theregistry there would shift the digest for essentially every workflow, which is
a much larger break and out of scope here.
effectbenchvendors its ownEffect(no new fields) and is unaffected;Effect.contract_hash()digestsare pinned unchanged by test.
Tests
tests/test_effect_verifier.py(+18) andtests/test_replayer_effects.py(+2):declared set with zero extras → CONFIRMED; declared set plus unintended records
→ REFUTED naming the count and the surplus (both the 1-extra and the 6-vs-37
shapes); a pre-existing in-scope record is NOT an extra; missing
identity_field→ structured refusal; unreachable baseline → INDETERMINATE +halt; repeated member; zero-member "adds nothing"; scope limiting; collateral
loss; contract-shape validators; param binding; and the back-compat regression
pinning pre-existing
contract_hashdigests andrequires_baseline.Verification
ruff check openadapt_flow— clean (CI-pinned 0.15.22)ruff format --check openadapt_flow tests— 492 files formattedmypy—Success: no issues found in 240 source filesmypystrict-safety file list —Success: no issues found in 27 source filespython scripts/check_consistency.py— passedpython scripts/check_release_consistency.py— passed (no payload-suffix fileadded, so
public-artifacts.jsonis untouched)python scripts/validate_claims.py— passedpytest -k "effect or verif or replayer or attended or receipt or contract or durable"— 1510 passed, 11 skipped.github/workflows/ci.ymlandpublic-artifacts.jsonare untouched.Co-Authored-By: Claude Fable 5 noreply@anthropic.com
🤖 Generated with Claude Code