Repository navigation
feat(conformance): fail a case whose server/time does not answer the client/time it received - #143
Merged
chrisuthe merged 4 commits intoOct 7, 2026
Conversation
…client/time it received Record each client/time the server received and each server/time it sent in a new time_exchange summary field, from the existing ControlMessageRecorder and the sendspin-go server adapter, and judge it on every case: the three server/time fields are integers, client_transmitted echoes an unanswered client/time, server_received is not later than server_transmitted, and no client/time was skipped or all left unanswered. The verdict applies to every scenario, so every scenario_revision moves up by one.
There was a problem hiding this comment.
🔵 Needs a closer look
The documented shutdown-related false positives require human review of the verdict’s acceptance criteria.
1 open finding
What changed in this PR
Adds clock-exchange checks across the conformance matrix, covering the message-exchange portion of #23 without judging client filter convergence.
Changes:
- Records ordered time exchanges in Python and Go server summaries.
- Validates timestamps, echoed requests, and unanswered requests.
- Updates contracts, tests, and all 13 scenario revisions.
| File | Description |
|---|---|
| tests/test_undeclared_formats.py | Updates summary fixtures. |
| tests/test_time_exchange.py | Adds verdict and recorder coverage. |
| tests/test_stream_start_gate.py | Updates summary fixtures. |
| tests/test_pcm_comparison.py | Updates summary fixtures. |
| tests/test_group_update_violation.py | Updates summary fixtures. |
| tests/test_format_priority.py | Updates summary fixtures. |
| tests/test_format_preference.py | Updates summary fixtures. |
| tests/test_first_metadata_state.py | Updates summary fixtures. |
| tests/test_encoded_audio_source.py | Updates summary fixtures. |
| tests/test_controller_state.py | Updates summary fixtures. |
| tests/test_chunk_framing.py | Updates fixtures and revision expectation. |
| tests/test_activation_violation.py | Updates summary fixtures. |
| src/conformance/scenarios.py | Bumps every scenario revision. |
| src/conformance/runner.py | Applies the verdict across scenarios. |
| src/conformance/protocol.py | Implements time-exchange validation. |
| src/conformance/adapters/aiosendspin_server.py | Includes exchanges in server summaries. |
| src/conformance/adapters/_aiosendspin_protocol_evidence.py | Captures transported time messages. |
| AGENTS.md | Documents the summary and revision contracts. |
| adapters/sendspin-go/server/main.go | Records requests and queued replies. |
| adapters/README.md | Documents evidence requirements and limitations. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…er kept sending Drop the clause that failed a server for answering no client/time at all: with none answered every one trails the last server/time, which the same verdict leaves unjudged, so it named a server whose only client/time arrived as the connection closed. An unanswered client/time now fails a case only on recorded evidence that the server went on without it: it answered one received later, or began sending another text message after receiving it. The recorder marks those sends in time_exchange as other-sent entries. No interval is measured.
…t requiring it RC1 describes the server answering a client/time and attaches no MUST or SHOULD to it. The unanswered reason now names that kind of statement. The check itself is unchanged.
A server shutting down may send what it had queued and close without answering the client/time it read last. Another message sent after a client/time now counts against the server only when a further client/time was received as well.
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.
Adds a verdict on the
client/time→server/timeexchange. Refs #23, and covers only the exchange shape: the convergence half of that issue is not externally assertable and is recorded as such below, so this does not close it.What is judged
A new server-summary field,
time_exchange, holds eachclient/timethe server received and eachserver/timeit sent, in order, as they went on the wire, with an{"type": "other-sent"}entry where the server began sending any other text message.time_exchange_violationinprotocol.pyfails a case when:server/timeomitsclient_transmitted,server_receivedorserver_transmitted, or carries one that is not an integer;client_transmittedis not the value of aclient/timethe server had received and not yet answered (the spec defines the field as the timestamp "received in theclient/timemessage");server_receivedis later than itsserver_transmitted. The spec does not state this as a rule; it follows from both being readings of the server's one monotonic clock, taken in that order;client/timewas left unanswered and the record shows the server went on without it: it answered one it received later, or began sending another text message after receiving it and then received a furtherclient/time. No interval is measured. The spec states this response declaratively ("Once received, the server responds with aserver/time",messaging.md:279) with no MUST or SHOULD, and the failure reason says so.A
client/timewhose ownclient_transmittedis not an integer fails the case naming the client. Anull, absent or unreadabletime_exchangefails as a harness gap. An empty list (the client sent noclient/time) gives nothing to judge.What is not judged, and why
server_transmittedwas stamped late enough. The spec requires it, but judging it needs the instant the frame left, which no summary carries.client/timereceived, and anyclient/timenothing was sent after. The spec gives no bound on the response, a connection can close with one still unread, and a server shutting down may send what it had queued and close. So a server interrupted by shutdown is not named.Known limits of the unanswered rule, also stated in
adapters/README.md:client/time, where the server sent no other text after the first, or where its adapter records noother-sententries. No evidence distinguishes those from a server that had no opportunity to answer, so this verdict is not complete coverage of "everyclient/timeis answered".client/time1,client/time2,server/time2 with noserver/time1 fails, though the spec does not define reply ordering. The record shows the server kept reading and sending, not that it had handled the first message, and wire order alone cannot tell a reply still pending from a message ignored. It fires on nothing in the matrix today.Evidence
ControlMessageRecorderis extended, with no second recorder. Received text is JSON-parsed (a test coversclient\/time). An unencrypted legacy connection bypasses the tapped transport and reportsnull.client/timein its own code, so itstime_exchangerecords a reply the adapter wrote, at the point it was handed to the library's send queue (there is no write hook), and records noother-sententries. Every sendspin-go server case already fails earlier on the missingserver/activate.scenario_revision
Clock sync is core messaging on every connection, whatever its roles, so the verdict runs on every case and all 13 scenarios move up by one from
mainat935b5ae(5,5,4,5,5,4,4,4,5,4,4,5,3 → 6,6,5,6,6,5,5,5,6,5,5,6,4).Expected cell movement
None. A full local matrix (macOS, 192 cases) against the live baseline: the same 32 cells pass, 0 regress, 0 newly pass. Across the 68 server summaries that reached
ok, everyclient/timewas answered and the verdict reported nothing; 51 of the 54 aiosendspin ones carryother-sententries. Because every revision changes, the regression check will print the whole matrix as exempt for this one release.Verification
python -m unittest discover -s tests: 331 pass (31 new intests/test_time_exchange.py).python scripts/run_all.py: report renders; read theserver-initiated-pcmaiosendspin → SendspinKit case page, which shows the recorded exchange in the server summary.