Repository navigation
examples: bind MCP retries to retained declaration snapshots - #423
Conversation
Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
There was a problem hiding this comment.
Reviewed the head 58e8659, the changes looks good, the example is clearly scoped (local formats, v0.2 record, not a v0.3 profile), the lost-response / retry / retained-snapshot story matches what #324 was asking to illustrate, and the tests also cover the important integrity paths well, also would not treat this as closing #324 as the normative design work still sits elsewhere, but as an informative example it is in good shape.
a few nits:
-
_commitment is independent of the generator helper, but both sides still go through the same rfc8785 library. A shared mistake in that layer would not be caught here. Fine for an example though.
-
snapshots = {digest(s): s for s in (before, after)} would silently collapse if two snapshots ever hashed the same. The current fixture differs on purpose, so this is not a bug but an explicit len(snapshots) == 2 (or an ordered list of pairs) will make the invariant easier to see
-
Numeric refusal is tested via load_json / canonical_bytes, not via digest() itself. Today it is fine because digest() calls canonical_bytes(). A later change that had digest() call rfc8785.dumps directly could slip past those tests.
-
Attempt order currently rides on dict insertion order. With Python ≥ 3.11 that is deterministic and correct here; wiring attempt 1 -> before and attempt 2 -> after through an explicit list would just be a bit easier to audit.
-
Path.write_text() with default newlines can turn \n into \r\n on windows, so the exact-byte SHA pins may fail there even when the JSON is fine. Prefer write_bytes(...encode("utf-8")) or write_text(..., newline="\n")
Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
|
Thanks for the careful review. Addressed in d51f443:
37 example tests and all 77 focused checks pass; the full host suite is 2,432 passed with six existing conditional skips. Ruff, formatting, prose, mypy and wheel/sdist build checks pass. The example remains informative; the remaining normative work in #324 is outside its scope. |
rajnisht7
left a comment
There was a problem hiding this comment.
Reviewed the head d51f443, the nits are addressed, portable UTF-8/LF writes via write_bytes, explicit capture ordering with a two-digest uniqueness assert, digest() covered in the numeric refusal tests, and the shared rfc8785 limit called out in the README and test comment. Still would not treat this as closing #324, but as an informative example this is in good shape.
|
@noah-ing this is approved and green; the only blocker is |
|
I ran one more focused pass on head The example declares The verification helper does something different: def _commitment(value, expected):
if value is None:
return "unavailable"
actual = "sha256:" + hashlib.sha256(rfc8785.dumps(value)).hexdigest()
return "matched" if actual == expected else "mismatch"That path recomputes raw RFC 8785 bytes without independently enforcing the example's narrower supported-value domain. A finite fractional number is valid RFC 8785 input, so a value the declared example format says is unsupported can still be classified by the verification path as Concrete counterexample shape: This is not a signature bypass and does not make the current fixed packet malleable: changing a committed integer to a fraction still changes the digest. The issue is narrower — the verifier can positively match evidence that lies outside the canonicalization contract it says it is verifying. The current numeric-refusal regression reaches A bounded fix would keep the recomputation independent of the generator helper, but give the verification path its own validation of the declared domain before calling
The decisive regression would be a float-containing retained object plus its otherwise matching RFC 8785 digest, which should report This seems worth fixing in the example because §3 of the draft already says unsupported canonical values must be reported rather than coerced/dropped, and the §6 acceptance table calls for unsupported numeric values to be rejected or reported unsupported. |
Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
|
Thanks both. Merged Agreed on the narrower domain mismatch: The 11 new cases failed before the fix and pass afterward, including 48 example tests, all 88 focused checks, and the full host suite (2,443 passed, six existing conditional skips) pass. Lint, formatting, prose, mypy and wheel/sdist builds pass. Current-head Python 3.11/3.12 CI and CodeQL are green; the gate awaits approval of the new head. The fix stays in the test helper and its documentation; no SDK or normative behavior changed. |
Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
|
Thanks. Merged current main ( All 48 example tests and 96 focused checks pass. The full host suite passed with 3,330 tests and 48 upstream conditional skips; lint, formatting, prose, mypy and wheel/sdist builds passed. Both generated output hashes are unchanged. Python 3.11/3.12 CI and CodeQL passed. GitHub reports no merge conflict; only current-head maintainer approval remains. |
Summary
Adds an executable worked example for the lost-response/retry/changed-declarations
case requested in #324. The session-wording work already landed in #413.
Run
python examples/mcp-retry/mcp_retry.py --out DIRto write one signed TRACEv0.2 record, its full transcript and declaration snapshots, and separate verification
inputs. The first attempt loses its response stream and stays unknown. Its retry
uses a new request ID and records success; an uncalled tool's declaration changes
between the two complete paginated captures.
The script is the single source; generated JSON is not committed. Tests run it in
two isolated directories, pin both outputs to their original byte hashes, verify
the record with a separately configured key, and recompute the full transcript
and snapshot commitments without the generator's helper. The test-only recomputation
checks the supported numeric domain before hash comparison and distinguishes
matched,mismatch,unavailableandunsupported. Both paths sharerfc8785;this is not independent validation of that canonicalizer.
No SDK, schema, dependency or acceptance-rule changes. The format and safe-integer
canonicalization are example-local, not an adopted v0.3 profile or live MCP
integration. Producer observations do not prove execution, authorization, server
identity, complete history, hardware provenance or exactly-once behavior. Remaining
design work in #324 stays outside scope.
Verification at
df63ca4mainat0014764, retaining every upstream changelog andexamples-index entry alongside ours. GitHub reports no merge conflict.
The example, its tests and the profile note are unchanged from approved
389459c.The numeric-domain regression was established at
389459c: 11 new cases failedbefore the helper fix and passed afterward, including fractional input supplied
with its matching raw-JCS digest.
Skips cover optional cross-repository dependencies, unavailable captured evidence
or verifier snapshots, and existing environment/vector conditions. No example
tests are skipped; this PR adds no skips or xfails.
was preserved and the previously approved implementation/tests were unchanged.
Current-head Python 3.11/3.12 CI
and CodeQL passed.
Each CI test job reports 3,330 passed and 48 conditional skips. The approval gate
requires a maintainer's approval of the new head. No local Windows or Docker
execution is claimed for this head. The earlier
58e8659verification includedfresh-install package checks and a locked-dependency audit; those remain prior-head
results.