Repository navigation
item 1: reconciliation hook - #1
Merged
Merged
Conversation
The v0.2 contract as a delta over v0.1, plus the scaffolding for items 1-8. Seven deliverables: a reconciliation hook, per-action effect:/resource: in policy, an EventSink protocol, ctrlrun inspect, an MCP gateway, a webhook approval provider, and an OpenTelemetry sink. Acceptance tests T13-T30. Written against MCP revision 2026-07-28, read rather than remembered. That revision removed the initialize handshake, protocol-level sessions and Mcp-Session-Id, and made Mcp-Method/Mcp-Name required headers that servers must validate against the body. The gateway accepts that revision and later ones only: it is the routing intermediary the validation rule exists for, and it parses every body rather than trusting a header it is meant to check. The core stays stdlib + pyyaml + click. gateway and otel extras are declared empty until the item that needs them lands.
Held reservations across MCP elicitation (§6.9). An input_required result now keeps the effect EXECUTING with an extended lease instead of going AMBIGUOUS, so concurrent duplicates stay blocked for the whole exchange and only the final result is mapped. Correlation is by params.requestState, which the 2026-07-28 revision makes optional; where the upstream omits it there is no identity for the exchange and the first leg stays AMBIGUOUS, because the only alternative correlator is a client-controlled field. Drops the mrtr policy key. Amends v0.1 §5.3 to permit extend_lease, narrowly: EXECUTING, same action_id, lease not already expired. Legacy revisions served in passthrough (§6.2). 2025-03-26 through 2025-11-25, relaying session ids, GET SSE streams and DELETE without interpreting them; header-body validation applies only where the headers exist. The earlier refusal over-applied a spec note that governs intermediaries whose enforcement is the header. This one decides from the parsed body on every revision. SSE resumption is not relayed for intercepted calls, which would otherwise deliver an outcome to a connection the gateway is not watching. Generalizes the FAILED mapping to protocol errors the spec defines as emitted before dispatch, and records the residual risks in THREAT_MODEL.md. examples/ and sector policy templates become build item 2 and the eighth deliverable; the build list renumbers and the release is item 9. Tests T26b, T30b, T31 added.
protect(..., reconcile=...) asks the remote what happened to an effect whose outcome is unknown. It is the second authority permitted to move a record out of AMBIGUOUS, and only where its answer points; every other reading of the answer — an exception, a nonsense return value, a hook that is never called — is "unknown", the value that changes nothing. Two trigger points, at most one call between them: blocking, when an AMBIGUOUS record refuses a new attempt's reservation, and eager, immediately after this attempt produced an AMBIGUOUS outcome. Eager never runs while a KeyboardInterrupt or SystemExit unwinds, for the reason v0.1 §5.5 gives, and never on a record this attempt could not mark. Amends SPEC-v0.1 §5.2 as SPEC-v0.2 §2.2 sets out. Adds RECONCILIATION_STARTED and RECONCILIATION_RESOLVED, and EFFECT_RESOLVED.data.resolved_by so evidence tells a human's judgement from a machine's answer. T13-T15.
Requiring it on the effect record means altering a table in databases that already exist, and v0.2 has no migration story; the event carries what the guarantee needs. Also states what the implementation does with a receipt after an eager reconciliation: nothing, for the same reason `ctrlrun resolve` does not edit one.
Completes the acceptance tests for SPEC-v0.2 §2. The implementation landed in 40ecd9d; this closes the three assertions §2 makes that no test pinned: - a hook that raises is logged with its exception and the caller still sees AmbiguousEffect, never the hook's RuntimeError (§2.4); - RECONCILIATION_STARTED precedes RECONCILIATION_RESOLVED precedes EFFECT_RESOLVED by event_id, and the retry's reservation follows all three, so the evidence reads in the order the reasoning goes (§2.5); - with no reconcile= the v0.1 path is untouched: the exact event sequence and receipt results of SPEC-v0.1 §5.4, with no v0.2 event type anywhere. The whole v0.1 effect suite runs unchanged alongside it. Also formats control.py, which went in unformatted and had `ruff format --check` failing in CI.
Mutation-testing SPEC-v0.2 §2 found three checks nothing was holding down: - §2.2, EFFECT_RESOLVED carries data.resolved_by: the reconcile side was pinned, the human side was not. T10 asserted `resolver` and stopped there, so dropping `resolved_by=RESOLVED_BY_HUMAN` from `ctrlrun resolve` stayed green. - §2.2, a resolution the store refuses is logged and dropped: no test opened that window. It is opened deterministically now — the hook resolves the record itself, the way `ctrlrun resolve` running during a slow query would. - §2.1, the hook is never asked about an action with no effect key: two defences guard this (the guard in `_Reconciler.available` and an assertion behind it), and defence in depth hid the mutation. The eager trigger is the only path that reaches it with no key, so the test drives exactly that; both defences were then mutated separately and each goes red. All 22 MUSTs in §2 now fail a named test when their check is removed.
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.
Build-list items 0 and 1. Neither was ever on the remote, so this PR carries both: the v0.2 contract, then the first thing built against it.
The contract (item 0)
docs/SPEC-v0.2.md— a delta over v0.1, plus the review resolutions that followed it. Two amendments matter beyond §2:resolved_byis an event field rather than a record column, because storing it on the record means altering a table in databases that already exist and v0.2 has no migration story.The hook (item 1)
protect(..., reconcile=...)asks the remote what happened to an effect whose outcome is unknown. It is the second authority permitted to move a record out ofAMBIGUOUS, and only where its answer points. Every other reading of the answer — an exception, a nonsense return value, a hook that is never called — is"unknown", the value that changes nothing.Two trigger points, at most one call between them:
AMBIGUOUSrecord refuses a new attempt's reservation, beforeAmbiguousEffectis raised;AMBIGUOUSoutcome, off by default.Both can arise in one attempt, so the budget is counted explicitly rather than inferred from control flow: a wrong or slow hook costs one call per attempt, and a retry loop cannot amplify it. Eager reconciliation never runs while a
KeyboardInterruptorSystemExitunwinds, for the reason SPEC-v0.1 §5.5 gives, and never on a record this attempt could not mark.Adds
RECONCILIATION_STARTEDandRECONCILIATION_RESOLVEDto the closed event set, andEFFECT_RESOLVED.data.resolved_byso evidence tells a human's judgement from a machine's answer.Tests
SPEC-v0.2 §10 T13–T15, parametrized over both
StateStoreimplementations. All 22 MUSTs in §2 were mutation-tested — remove the check, confirm a named test fails, restore — and three had nothing holding them down:EFFECT_RESOLVEDcarriesdata.resolved_byEach now fails a named test when its check is removed. The last needed a deterministic test driving the one path that reaches reconciliation with no effect key (the eager trigger); both defences were then mutated separately.
Also
control.pywent in unformatted, soruff format --checkwas failing in CI onmain. Formatted.Verification
725 tests pass,
mypy --strict src/andruff checkclean,ruff format --checkclean,ctrlrun demoruns the four scenarios in 0.07s with no network. Every SPEC-v0.1 §7 acceptance test still passes unchanged.