Repository navigation
test(tasks): add bounded CB/CAN evidence fixtures for #567 - #797
solloek369-arch wants to merge 2 commits into
Conversation
Signed-off-by: Loek <solloek369@gmail.com>
dinakarjs
left a comment
There was a problem hiding this comment.
Independent bounded review of 1206ad2 under Linux/Python 3.12.14 with the repository's hash-locked dev environment: tests/unit/task_evidence567 gives 360 passed. I independently replaced the acknowledged notification's params with the requested listen params; the focused suite gives exactly 1 failed, 359 passed, exposing requested-but-not-accepted as intended. Source restored byte for byte afterward. An earlier mutation invocation resolved the venv symlink incorrectly and could not import pytest; it is not the mutation result cited here.
The code and README consistently keep role authority/population completeness as independently stipulated synthetic premises, distinguish exact-ack exclusion from whole-request exclusion, and preserve operand binding as a separate prerequisite from joint context coverage. No additional source blocker found in this slice. This does not implement runtime callback authentication, replay ordering or terminal-effect verification.
Current hosted CI fails at Security scan before the unit matrix executes; I have not labeled those skipped tests green or diagnosed the scan from a step name alone. Retain normal current-head CI and maintainer approval gates. Reviewed with OpenAI Codex.
|
CI follow-through: I read failed Ubuntu/Python 3.11 job 119756976070. Security scan reports certifi 2026.7.22 / PYSEC-2026-4320, fixed version 2026.10.19, from the inherited dev lock. This is the dependency update already submitted in #795, rather than a failure of the new CB/CAN fixtures. The hosted tests remain unexecuted in that run; my 360-test local result is separate. Please refresh current-head CI after the dependency repair lands. No scan or approval gate was bypassed. Prepared with Codex. |
carloshvp
left a comment
There was a problem hiding this comment.
Reviewed exact head 1206ad22949c4e31d041fb82cda2ac5a3545c1f5 in an isolated checkout.
Approving the bounded, test-only CB/CAN fixture slice. The source keeps the acknowledged task list separate from the requested list, requires independently pinned role authority, and preserves authentication, operand binding, joint applicability and accounting as separate prerequisites.
Independent validation: the complete task_evidence567 selection passes 360 tests; repository Ruff passes. In a separate process I substituted the requested listen parameters for the acknowledgement parameters. Exactly the requested-but-not-accepted regression fails (1 failed, 359 passed), while its positive twin stays valid. The checkout was unchanged by this trial.
The synthetic authority and fixed-population premises remain explicit; these fixtures do not establish production callback authentication, replay ordering or terminal effects. The failed hosted Ubuntu job reports the inherited certifi 2026.7.22 audit finding before the unit matrix executes; #795 carries the separate lock repair. Hosted cancelled jobs remain unvalidated. All PR commits carry DCO sign-off.
Reviewed with OpenAI Codex.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Reviewed the CB/CAN fixture slice on 1206ad2,a few points needs to be addressed:
- CB-06 does not inspect the actual subscriptions/listen taskIds. A valid client + valid permission for T still returns ESTABLISHED even when the listen request asked for a different task or none. Missing negative test.
- CAN-01/action is hard-coded to "tasks/cancel" and never compared to the real request.method. A tasks/update request with an action witness still saying "tasks/cancel" makes the named CAN-01/action relation ESTABLISHED (composite correctly fails via policy).
- CB-05 never checks the listen request’s own taskIds or jsonrpc == "2.0". listen.taskIds=[] + ack containing T, or missing/"1.0" jsonrpc, still yields ESTABLISHED.
- dict(evidence.bindings) silently collapses duplicate operand names. No cardinality/uniqueness check on the five CAN-01 operands.
ruff import grouping + blank-line rules also fail on test_callback_cancellation.py.
drk-ram
left a comment
There was a problem hiding this comment.
Reviewed head 36b29dd13a29ac67fdb14f430641b5ae003805fc.
Requesting changes. The slice is still the right bounded fixture, and I am not asking it to become a Tasks runtime. Two of the named relations can be established from evidence that does not bind the claim, and the native-shape predicate does not require JSON-RPC 2.0.
Local validation on this head, Python 3.11.9, repository Ruff config: pytest tests/unit/task_evidence567 -q gave 360 passed. ruff check on the three new Python files passed under the project's [tool.ruff.lint] select, so I am not reproducing an import-grouping failure against this repo's config. Hosted CI on this head is green; I am not treating that as a substitute for the oracle gaps below.
1. CAN-01/action is compared to the literal "tasks/cancel", not to the captured method.
In cancellation(), method is taken from the request and passed to evaluate_authority, but the operand table is:
values = {"principal": profile.client, "action": "tasks/cancel", "resource": task, ...}Resource is request-derived. Action is not. I rebuilt a tasks/update request, signed the five operand witnesses over that request, and left the action witness at "tasks/cancel" (what cancel_evidence always writes). Result on this head:
CAN-01/action:ESTABLISHED/individual_exact_operand_bound- policy:
NOT_ESTABLISHED/outside_fixture_action_scope - composite:
NOT_ESTABLISHED
The composite fails, so this is not a false grant of cancellation authority. It is a false establishment of the named operand. That matters here because the fixture's own contract, and the omit-one tests, treat each operand as an independent prerequisite. A policy miss must not be what hides an action relation that never looked at request.method. draft-durga-agenttrust-trace-verification-00 is the same rule in the trace setting: a named claim is established only when the submitted evidence binds that claim, not when some other check happens to fail closed. Please compare the action witness to the captured method (and add the tasks/update negative, with the witnesses re-signed over that request). Reusing witnesses bound to a different occurrence only proves the subject check, which I confirmed separately; that is not this bug.
2. dict(evidence.bindings) keeps the last witness for a repeated operand.
supplied = dict(evidence.bindings) drops every earlier witness for the same name. I prepended a contradictory CAN-01/action witness (value="tasks/update") and left the valid witness last. The named action relation and the composite both came back ESTABLISHED. The accounting layer in this same slice refuses to collapse a valid-plus-malformed duplicate into a pass. The relation evaluator should not last-win either. Require exactly one witness per operand name, or treat a repeated name as not established. A negative that supplies two action witnesses must not stay green.
3. The native-shape check does not require jsonrpc == "2.0".
subscription_acceptance checks method and the presence or absence of id. It does not check the version. A listen/ack pair with jsonrpc "1.0" on the request and no jsonrpc on the notification still returned ESTABLISHED under the reason that claims a native listen and acknowledged notification. Ext-tasks 7b8e2bd messages are JSON-RPC 2.0. Please reject a missing or non-"2.0" version as not established, with a negative next to the existing response-versus-notification case.
What I am not asking you to change:
- CB-05 must not also require the listen request's
taskIdsto contain T. I reproducedlisten.taskIds=[]plus an ack that contains T, and the relation stayedESTABLISHED. That is the distinction this PR exists to hold: acceptance is the complete acknowledged list, not the requested list. Folding the request list back into CB-05 would make therequested-not-acceptedcontrol vacuous in the other direction. - CB-06 staying
ESTABLISHEDwhen the listen asked for a different task is consistent with the approved profile and with ruling B. A task id is a lookup key, not authority. I reproduced that case. Please leave it as a documented limit of the authority relation, not a silent listen-contents check. I do not want the permission witness replaced byparams.notifications.taskIds.
The synthetic role keys and the frozen population remain premises, not production callback authentication. This still does not close #567. The three oracle fixes above are what I need on the next head.
dinakarjs
left a comment
There was a problem hiding this comment.
Follow-up on exact head 36b29dd13a29ac67fdb14f430641b5ae003805fc: I independently reproduced the three defects identified in the current reviews. This supersedes my earlier “no additional source blocker” assessment.
- A captured
tasks/updaterequest with all witnesses re-signed over that request can still establish CAN-01/action from a witness sayingtasks/cancel. Composite authority is not established because policy rejects the method; the named operand is nevertheless wrong. - A contradictory duplicate operand prepended before the valid one is silently collapsed, leaving composite authority established.
- A listen/ack pair with a non-2.0 or missing JSON-RPC version can establish the native acknowledgement relation.
The original 360 cases plus one positive control pass; the three initial independent negative controls fail. Current-head hosted CI is green, but does not detect these gaps.
A tested repair is ready for author adoption: dinakarjs#12, commit fb107c788432e73a789a687e34beef580e3d9d62, directly based on this head. It changes only the existing fixture oracle, tests and README. It preserves acknowledgement-list acceptance separately from subscriber authority.
Validation: 403 focused cases pass; keeping these tests but restoring the original fixture implementation yields 30 failures / 373 passes. Full unit suite: 2,892 passed, 8 skipped; repository Ruff, mypy, configured Bandit and dependency consistency pass. Environment setup and limitations are recorded in the companion.
Please adopt the repair into this existing PR, rerun hosted checks and obtain independent re-review. I contributed the proposed fix, so my validation is not independent approval of that fix. Synthetic authority/population premises remain; this does not implement Tasks runtime or close #567.
Reviewed and prepared with OpenAI Codex.
What
Smallest acceptance counterexample: a
subscriptions/listenrequest asks for task T, but its separately authenticated, exactly bound completenotifications/subscriptions/acknowledgednotification accepts no tasks. Reading the requested list instead of the acknowledged list would incorrectly establish acceptance. This test-only continuation of #701 distinguishes those worlds; it does not implement runtime callback authentication or Tasks.Expected and measured behavior
The positive acknowledgement includes T:
ESTABLISHED. The complete negative acknowledgement excludes T:VERIFIED_CONTRADICTIONonly for acceptance in that acknowledgement, not for the whole request or any other subscription. Missing or inapplicable acknowledgement evidence isNOT_ESTABLISHED.In
test_callback_cancellation.py, the exact controls are:test_cb05_exact_native_ack_only[positive-ESTABLISHED]test_cb05_exact_native_ack_only[requested-not-accepted-VERIFIED_CONTRADICTION]Both pass. Weakening
subscription_acceptanceto taketaskIdsfromrequest["params"]["notifications"]instead of the separate acknowledgement makes the negative assertion fail: actualESTABLISHED, expectedVERIFIED_CONTRADICTION. The positive twin remains green; the full focused mutant run gives 1 failed, 359 passed. Byte-exact restoration returns 360 passed. This was reproduced on both cold-review Python versions below. The consequence is a load-bearing distinction between requesting a subscription and evidence that this acknowledgement accepted it—not a production defect claim.Four additive test/documentation files
All are under
tests/unit/task_evidence567/:callback_cancellation.py: conditional CB/CAN relation helpers, reusing existing signed-witness verification, Boundary/Cedar evaluation and accounting.test_callback_cancellation.py: 98 relation/integration cases with established positive twins and decisive negative controls.test_callback_accounting.py: 150 completeness/accounting cases.CB_CAN_README.md: approved profile, trust premises, evidence boundaries and limitations.No existing file is changed. The original #701 README, eight Python files and A–G runner remain byte-identical. Its open production callback-authentication/authority work is not contradicted by these synthetic fixtures.
Why
Imran accepted the matrix and completeness contract and authorized fixture code, with two decisions:
subscriptions/listenfirst, against ext-tasks at7b8e2bd; CB-05–07 are included, webhook waits.The approved matrix is covered as separate relations:
Why 150 accounting cases
There are fourteen fixed obligations: seven per captured delivery and seven per captured cancellation request (five CAN-01 operands, CAN-02 and composite authority). The 150 cases comprise 140 per-boundary parameter cases plus ten population/scope controls. They reuse #701's count-before-eligibility mechanism while testing this slice's source triggers: omission, wrong occurrence/boundary, duplicates, malformed attempts and valid-plus-malformed duplicates stay visible. An explicit
NOT_ESTABLISHEDcan be accounted without becoming established; an empty or supporting-only population is outside the bounded contract, not a vacuous pass. The frozen source population is a fixture premise, not proof of complete real traffic capture.Security impact
None to production code or security boundaries. Synthetic deterministic test keys and their role authority are explicit fixture premises, not real credentials. No dependencies, production files or wire schemas change; existing #701 behavior and mutation-runner interface remain unchanged.
Compatibility/limitations: no live callback authentication, Tasks runtime/storage, real delivery, cancellation effect, terminal-result verification, external effects, replay/out-of-order semantics, webhook profile or final wire representation is implemented or established. Evidence validity is scoped to the selected fixture context; this introduces no new per-delivery reauthorization requirement. This contribution does not close #567.
Test plan
PYTHONPATH=src python -m pytest tests/unit/task_evidence567 -q— 360 passed on Python 3.11, 3.12 and 3.13.tests/unit/task_evidence567/), mypy (src/cmcp_runtime/) and configured Bandit (src/) — passed.subscription_acceptanceto read task IDs from thesubscriptions/listenrequest instead of the separately boundnotifications/subscriptions/acknowledgednotification. The testtest_cb05_exact_native_ack_only[requested-not-accepted-VERIFIED_CONTRADICTION]then fails (actualESTABLISHED, expectedVERIFIED_CONTRADICTION). The positive twin remains green. Full focused run: 1 failed, 359 passed. Restoring the check returns 360 passed.certifi/pip-audit issue is addressed in PR #795.DCO sign-off
Developer Certificate of Origin (https://developercertificate.org).
Signed-off-by: Loek solloek369@gmail.com
Commit:
1206ad22949c4e31d041fb82cda2ac5a3545c1f5.