recorded(match): evaluate on the engine (verify includeRequests) so repeated-key and JSON-body array rules (rift#1553/#1579) match #337

Activity

1. EtaCassiopeia

   added

   agent-foundFinding discovered by automated agent reviewFinding discovered by automated agent review

   needs-triageIssue awaiting triage and prioritizationIssue awaiting triage and prioritization

   on Oct 18, 2026
2. EtaCassiopeia commented on Oct 18, 2026

   

   EtaCassiopeia

   on Oct 18, 2026

   CollaboratorAuthor

   More actions

   achird-labs/rift#1579 (PR #1630, merged) has landed the body half. Engine semantics for verify/PredicateEvaluator.java:178-224:

   • equals, contains, startsWith, endsWith and matches: an expected array is a subset, in any order. A scalar or object expectation matches any element. An object against an array of objects matches when one element satisfies the whole object.
   • Top-level expected array: compared by position.
   • deepEquals: compares arrays as multisets and reads null as "null". Under equals, null matches only null.
   • Missing field: reads "".
   • Numbers: compared by their JavaScript spelling (1 equals 1.0).

   This requires the engine release that ships #1553 and #1579.
3. EtaCassiopeia commented on Oct 27, 2026

   

   EtaCassiopeia

   on Oct 27, 2026

   CollaboratorAuthor

   More actions

   Triage: build it. Unblocked: rift#1553 (#1628) and rift#1579 (#1630) both shipped in v0.24.0, the pinned engine (#346)

   Root cause

   Imposter.recorded(RequestMatch) and Space.recorded(RequestMatch) (ImposterImpl.java:252, SpaceImpl.java:65) fetch the whole journal and filter it in the JVM with verify/PredicateEvaluator. That is a second implementation of Mountebank's predicate semantics, and it has to track every engine change by hand. It has fallen behind three times: #330 and #335 (XPath node values, rift#1495), and now this issue.

   On 0.24.0, the client-side evaluator disagrees with the engine on more than repeated query keys:

   Case	Engine 0.24.0	PredicateEvaluator
   equals {"query": {"k": ["a","b"]}} vs ?k=b&k=a	match (subset, any order)	no match (the array is compared as the text ["a","b"])
   deepEquals query array	multiset	no array branch
   array expectation on a repeated header	match	no array branch
   equals {"body": {"a": 1}} vs {"a": 1.0, "b": 2}	match (field subset, JS number spelling)	no match: the object is serialised and compared to the raw body text (evalScalar → jsonToCompareString)
   body arrays: subset, scalar-vs-any-element, object-vs-array-of-objects	Mountebank rules	none
   deepEquals null vs "null", missing field reads ""	Mountebank rules	not modelled
   inject predicate	evaluated	throws InvalidDefinition

   Porting all of this (about 800 lines of rift's predicates/json.rs plus arrays.rs) gives a fourth copy of the rules: rift, rift-scala's RequestMatcher and this one. Each new engine change would produce another bug like this one.

   Decision: make recorded(match) engine-authoritative, as verify already is (#127)

   The engine already answers this question exactly. POST /imposters/:port/verify with "includeRequests": true returns:

   • every recorded request that satisfies all the predicates;
   • in journal order, with no cap;
   • evaluated by the same evaluator that serves traffic (rift-mock-core/src/imposter/core/verify.rs, verify_with).

   It is available on every transport: remote and spawn use RemoteTransport.verify, embedded uses the FFI rift_verify. The SDK already builds that body (VerifyBody.build(match, flowId, VerifyDetail.REQUESTS)) and reads the reply (VerificationResult.requests()).

   Rejected alternative: port #1553 and #1579 into PredicateEvaluator, using the engine's 104 differential cases (conformance/differential/cases/repeated_query.json, body_arrays.json) as vectors. It would work, but it keeps the drift problem and still leaves inject unsupported.

   Implementation

   1. ImposterImpl.recorded(RequestMatch):
      return VerificationResult.read(transport.verify(port, VerifyBody.build(match, Optional.empty(), VerifyDetail.REQUESTS)), VerificationTimes.atLeast(0)).requests();
      Factor out a small private helper if verifyResult can share it. Do not call requireRecording(): a non-recording imposter keeps today's contract, an empty list. The engine counts {0,0} for it, so no extra definition() round trip is needed.
   2. SpaceImpl.recorded(RequestMatch): the same call, with the space's flowId (as SpaceImpl.verifyResult already does). spaceRecorded + client filter goes away.
   3. PredicateEvaluator:
      • Mark it @Deprecated(since = "0.3.8", forRemoval = true). Its javadoc should point to Imposter.recorded(RequestMatch) and verifyResult(match, VerifyDetail.REQUESTS).
      • Remove both internal call sites.
      • Leave its logic unchanged; no array port.
      • It is public API. rift-scala lists it as ExcludedClass in FacadeCoverage.scala:838 (never called), so deprecating it breaks nothing downstream.
      • Delete it in the next minor release.
   4. Javadoc on Imposter.recorded(RequestMatch) and Space.recorded(RequestMatch):
      • The engine evaluates the predicates, so the result follows the running engine's rules: Mountebank's array rules for query, header and body from rift 0.24.0, plus inject predicates.
      • It costs one admin call instead of a journal fetch.
      • A malformed predicate (bad regex or xpath) fails with the engine's error, as verify does.
   5. Docs: docs/design/sdk-api.md, in the verify section: recorded(match) is engine-evaluated; there is no client-side matcher.

   Gate

   • Unit tests (ImposterImplTest / SpaceImplTest, using a fake transport):
     • recorded(match) sends exactly one verify call whose body is {"predicates":[…],"includeRequests":true} (literal JSON).
     • It sends no journal fetch.
     • It returns the reply's requests in order.
     • The space variant carries flowId.
   • Conformance IT RecordedMatchArraysIT (all lanes):
     • Record ?k=a&k=b, ?k=b, a JSON body {"items":[{"id":1},{"id":2}],"n":1.0}, a body without n, and a repeated X-Tag header.
     • Assert literal counts for:
       • equals query k:["b","a"] → 1
       • deepEquals query k:["a","b"] → 1
       • equals query k:"b" → 2
       • equals body {"items":[{"id":2}]} → 1
       • equals body {"n":1} → 1
       • equals body {"n":""} → 1 (missing field reads "")
       • equals headers X-Tag:["x","y"] → 1
   • Keep XPathRecordedMatchIT unchanged. It must still pass, and now does so through the engine.
   • Embedded lane: confirm the rift_verify path returns requests (EmbeddedTransport.verify).

   Out of scope

   Removing PredicateEvaluator (next minor), and rift-scala's own RequestMatcher (a rift-scala issue, if wanted).

   Model: Opus. Size: about 60 lines of production code, plus tests.
4. EtaCassiopeia

   added

   ready-to-implementTriaged: root cause confirmed, design and implementation steps recordedTriaged: root cause confirmed, design and implementation steps recorded

   bugSomething isn't workingSomething isn't working

   and removed

   needs-triageIssue awaiting triage and prioritizationIssue awaiting triage and prioritization

   on Oct 27, 2026
5. EtaCassiopeia

   changed the title [-]adopt rift#1553: client-side PredicateEvaluator has no array branch for repeated query keys (and #1579 body arrays)[/-] [+]recorded(match): evaluate on the engine (verify includeRequests) so repeated-key and JSON-body array rules (rift#1553/#1579) match[/+] on Oct 27, 2026
6. EtaCassiopeia

   closed this as completedin #350on Oct 28, 2026