[rmcdaniel]

Part of #325. Follows #849, and closes the named-argument gap its review found.

What changes

For a declared query, published Server checks the arguments with Workflow's WorkflowQueryContract::validatedArgumentsForRun:

• Named arguments. A non-empty map with no key PHP casts to an integer is taken as named arguments. Any other map (empty, or with a canonical integer key) decodes to an AvroMapValue object, not an array. At the top level it is refused with "must decode to an array"; nested, it fails array, passes object, and stays one variadic argument.
• Positional arguments. These fill the parameters in order. A variadic parameter takes the rest, and extra values are reported.
• Missing arguments. These take their default, or are reported as required.
• Values. A value must match its declared type: nullable, union, intersection, int, float (accepts int), string, bool, array, iterable, scalar, true, false, null, mixed, object or a class name. A null must be allowed.
• Refusal. Any violation answers 422 invalid_query_arguments, with "Workflow query [] argument validation failed." and validation_errors by argument.
• Worker. Otherwise the worker receives the normalized positional list, encoded again.
• Registered defaults. A default the worker registers as JSON takes PHP's array shape. Canonical integer keys 0 to n-1 make a list; string keys make a map. Any other integer key fails PHP's Avro encoder with an uncaught 500, and native answers 500 query_default_unencodable, naming the parameter.
• No parameter contract. A declared query without one takes a list as given and refuses named arguments ("Named arguments require a durable or loadable workflow query contract.").

Before this change native:

• checked positional arguments only;
• refused a map as "must decode to an array";
• reported every violation as a bare invalid_query_arguments;
• refused defaults and most declared types as unsupported_query_contract;
• passed the client's blob through unchanged.

Native now:

• ports the rules in query_contract.rs;
• returns the normalized list or PHP's refusal from check_query_contract;
• enqueues the re-encoded blob from query_attempt;
• decodes and writes arguments with map entries in wire order (codec::OrderedValue). PHP spreads a variadic named map's values in wire order, and Apache's generic reader keeps maps in a hash map. codec/wire_order.rs reads the datum that the public reader has already validated a second time, through a private binding schema. That schema spells the map as an array of {key, value} records, as PHP's ValueDatumWriter does: the same bytes, and Apache still owns every primitive and block. Writing goes through the same binding, under the public header, and the result is checked again by the public reader.

Kept, and documented in the checkpoint:

• callable matches nothing. Neither map adapter is invokable, and PHP's is_callable also accepts strings naming functions, which a native server cannot resolve.
• Default key order. A registered default map keeps sorted key order, not the registered order: native's JSON objects don't preserve order. For the same reason {"1":"a","0":"b"} is a list on native, where PHP fails to encode it.
• Class names. A class name a published server can load would refuse other objects. Native, loading no classes, accepts any object.

Contract

The new query-contract-arguments fixture runs over raw HTTP on protocol 1.20 in a fresh namespace. The worker declares state(string $prefix), greet(string $prefix = 'dflt') and plain. Twenty-two cases are recorded against published 2.11.5:

• Routed, with the worker's received blob checked: named and positional state, greet with nothing (default filled), named greet, and a list to plain.
• Refused, with messages checked: unknown name, missing required, wrong type, null, too many, and named arguments to plain.
• Map adapters (from review): an empty map, {"0":"ok"} and a mixed-key map are refused as not arrays. A nested empty map is refused by arr(array), but routed whole by obj(object) and the variadic rest(mixed ...).
• Wire order (from review): {value: {z: 20, a: 10}} reaches rest as [20, 10].
• Registered defaults (from review): {"0":"x","1":"y"} reaches the worker as [["x","y"]]. {"0":"x","k":"y"} and {"1":"x"} answer 500; the contract pins only that status, since PHP's body is the generic "Server Error".

Embedded applications have no worker protocol, so the case is explicitly inapplicable there. Contract: query-contract-arguments-contract.mjs. Its unit test is registered in server-parity.yml. The port log's open "Named and validated query arguments" entry from #849 is closed.

Verification (run locally on the Rust dev host)

• Fixture: recorded against published 2.11.5 on fresh state, and against a native image built from this branch. Both pass.
• Native tests (SQLite, in this branch's own cargo target directory):
  • cargo test --test execution: 623 passed, 1 failed. The failure is updates_have_no_lifetime_cap_and_preserve_accepted_work_after_wait_timeout, which ran while an image build loaded the host. It lacks a worker heartbeat and outlasts the staleness window under load; Rust #325: accept completions and parallel groups over 100 #851 fixes the test. It passes alone and does not use the changed code.
  • cargo test --lib: 358 passed, including round trips of 7 published blobs byte for byte, a repeated-key test and the default-shape test.
• New tests: query_contract_arguments.rs (normalized routing; refusals with messages) fails on Rust #325: route query arguments the server cannot decode #849's source. query_contract.rs unit tests cover the type rules and integer-keyed maps. The existing query tests pass (32).
• Node: the contract, runner and fixture-selection unit tests pass (293).
• Not run locally: PostgreSQL and MySQL; CI runs them.

Checkpoint: docs/rust-server-port-checkpoints/2026-10-28-query-contract-arguments.md.

🤖 Generated with Claude Code

[rmcdaniel]

Sonar's two rust:S3776 findings are fixed at 5f6832dd:

• query_attempt (31): the re-encode moved into routed_arguments.
• query_contract::validate (47): split into positional, named, spread and missing.

Behavior is unchanged. A new unit test, variadics_take_the_rest_or_a_named_list, guards the variadic paths through the split. Locally: query_contract unit tests 5/5, query execution tests 34/34.

[rmcdaniel]

At 5f6832dd63ca40ba9889f3c01acef2e06332c969, the new query normalizer needs a correction for PHP's Avro map adapters before merge.

Arguments::from_value treats an empty map and a map with contiguous numeric keys as positional arrays. Published Core 2.15.1 does something different: Avro::fromDatum returns AvroMapValue for an empty map or any map containing a key that PHP casts to an integer. That adapter is a plain object, so the published Server broker rejects it before parameter normalization with 422 invalid_query_arguments, message Workflow query [<name>] arguments must decode to an array., and validation_errors.arguments = ["Query arguments must decode to an array."].

Concrete admission cases:

• greet(prefix = 'dflt') with an empty Avro map must fail the array-shape check. The current native code converts it to [], fills the default and routes it.
• state(prefix: string) with Avro map {"0": "ok"} must also fail that check. The current native code converts it to ["ok"] and routes it.
• A map mixing a numeric key with a named key also becomes an adapter in PHP; treating it as named arguments changes the validation stage and response.

The same distinction matters inside a valid named-argument map. I ran the published Core codec and normalizeNamedArguments with a nested empty Avro map:

Parameter	Published PHP result	Current native source
array $value	Type validation error	Accepts every Value::Map as an array
object $value	Accepts one AvroMapValue	Recognizes objects only as Value::Bytes
mixed ...$value	Keeps one AvroMapValue argument	Spreads the empty map into zero arguments

Please preserve this map-adapter distinction in admission, type checks and named variadic expansion, replace the contrary integer_keyed_maps_are_lists expectations, and add differential HTTP cases for the outer and nested forms. A fix limited to outer admission leaves the nested acceptance and dropped-argument problems intact.

Verification scope: the executed controls used exact Core 07e0d0e1a1185bd0a3bbd4e3fb56511cc57ca9bc, PHP 8.3.36 and Apache Avro 1.12.2. They confirmed empty, numeric-key and mixed-key maps decode to AvroMapValue, ordinary string-key maps decode to PHP arrays, and the three nested contract results above. Server 2.11.5 locks Apache Avro 1.12.1. I inspected its broker source; these controls are not an installed Server HTTP or native Rust differential run.

Additional key-casting control: on PHP 8.3.36 with 64-bit integers, array_key_first([$key => true]) casts "0", "1", "-1", "9223372036854775807" and "-9223372036854775808" to integer keys. It preserves "", "00", "01", "-0", "-01", "+1", "1.0", "1e0", " 1", "1 ", "9223372036854775808" and "-9223372036854775809" as strings. All 17 cases were executed in an isolated PHP control. The map-adapter predicate therefore needs canonical signed-integer spelling and range, with empty-map handling separate; a general numeric-string predicate changes PHP's validation behavior. This additional control exercised PHP key casting, not Server HTTP or Rust.

There is also a positional-order correction for ordinary string-keyed maps used as named variadics. At this same head, queries.rs decodes through ValueCodec::decode; Value::Map uses BTreeMap, and spread iterates its sorted values. For items(int ...$rest) with input ['rest' => ['z' => 20, 'a' => 10]], the published Core codec/normalizer preserves keys ['z', 'a'] and produces [20, 10]. The current native path produces [10, 20]. This changes the positional arguments delivered to the worker.

I executed that PHP round-trip/contract control against exact Core 07e0d0e1, PHP 8.3.36 and Avro 1.12.2: the deliberately unsorted string map normalized to [20, 10], a named list control did the same, and a numeric-key map for mixed ...$rest remained one AvroMapValue with no validation errors. Please preserve incoming map order until after variadic expansion and include the unsorted string-map case in the differential HTTP fixture. The native mismatch here is established by source inspection; no native or installed Server HTTP run is claimed.

One refinement to the documented callable limitation: exact-Core PHP controls reject both AvroBinaryValue::fromBytes('x') and an empty AvroMapValue for a callable $value parameter with The value argument must be of type callable. Both adapters are plain, non-invokable objects. A 'strlen' string control passes in PHP. Distinguishing callable from generic object can therefore avoid the known adapter false positives while keeping dynamic function/class lookup explicitly limited. These three controls used the same exact Core/PHP/Avro dependency scope above; no native or Server HTTP run is claimed.

Re-review at d376fe6b5a706258c291220e646b9c7abf7b8e6a: the original adapter-shape finding is resolved in source. I compared the exact file contents, checked admission/object/array/variadic branches, matched integer_key to the 17-case PHP oracle, and inspected the six new fixture expectations. The author's differential receipt reports 18/18 cases against both published 2.11.5 and native, plus 623 execution/353 library tests. The separate unsorted-map variadic order finding remains: this head still decodes into BTreeMap and iterates sorted values. Please retain the review hold for that correction and its differential case. This re-review adds source/fixture inspection, not an independent Rust or HTTP execution.

Additional default-conversion case on afa2adbb9c70fe8784991fa49a94af93c4773b57: register an optional array $value parameter with default_available: true and raw JSON default: {"0":"x","1":"y"}, then query with an empty positional list. PHP's request JSON decoding converts the default to a PHP list. Published WorkerController::commandHandlerParameters preserves the default, as does Core's durable-contract normalizer. My exact-Core PHP control produces arguments [["x","y"]] and an Avro array for the inner value. Native worker_contracts.rs clones the raw JSON parameters, while query_contract::from_json turns every nonempty object into OrderedValue::Map; the worker consequently receives an Avro map adapter instead of that default array. Please include a raw-registration differential case for this boundary.

Related controls under the same Core 07e0d0e1 / PHP 8.3.36 / Avro 1.12.2 scope: an empty default object becomes [] (matching the native special case); a named default object {"z":20,"a":10} retains PHP key order; a mixed numeric/named default reaches normalization but PHP encoding refuses it with invalid_map_key. The native dependency currently lacks serde_json's preserve_order feature, so JSON-default order needs its own disposition alongside the incoming-Avro order correction. These are executed PHP controls plus exact-source tracing, not an installed Server or Rust differential result.

Re-review of exact bc2b1cf0a2ac77433c5676e98dc1d424bba0e697: the order-preserving binding now delegates binary decoding/encoding to Apache's GenericDatumReader/GenericDatumWriter, checks the public schema/frame before decoding, checks truncation/trailing bytes on the binding pass, and validates the written public frame. This resolves the custom-binary-parser concern. The vector-backed query path and unsorted_variadic fixture address the original variadic order issue; the plain-adapter callable false positives are also removed. The JSON-default case above remains for qualification.

There is a new complexity correction in wire_order::from_binding: every map record performs entries.iter_mut().find(...), including each distinct key. Converting a map with n distinct entries therefore performs n(n-1)/2 key comparisons; 2,000 entries require 1,999,000. Preserve the vector's order and PHP's first-position/last-value duplicate behavior with a standard key-to-slot index instead of repeatedly scanning the vector. This is an exact-source complexity finding, not a measured runtime claim. The reported 19-case differential pass and 357 library tests are attributed evidence; the full local execution run remains 622 passed / one timing failure with three isolated passes, not a full-suite pass.

[rmcdaniel]

Addressed Cloud's AvroMapValue review (6485312975) at d376fe6b.

What changed in query_contract.rs:

• Top level. A map is named arguments only when it is non-empty and has no key PHP casts to an integer. Any other map is an AvroMapValue object and is refused before normalization, with "must decode to an array".
• Types. array and iterable accept only real PHP arrays. object and class names accept bytes and adapter maps.
• Variadics. A named variadic spreads only a PHP array; an adapter map stays one argument.
• Tests. The contrary integer_keyed_maps_are_lists test is replaced by empty_and_integer_keyed_maps_are_objects_not_arrays, and a_nested_adapter_map_is_an_object_and_one_variadic_argument is new.

Differential cases added to the fixture, recorded against published 2.11.5:

• greet with an empty map, state with {"0":"ok"}, and a mixed-key map: each 422 ... must decode to an array;
• {value: {}} to arr(array $value): "The value argument must be of type array.";
• the same to obj(object $value) and rest(mixed ...$value): routed, and the worker receives [{}].

Verification on the Rust host. The fixture passes against published 2.11.5 (fresh state) and against a native image built from this head, 18/18 cases each. SQLite execution 623 and lib 353 pass.

[rmcdaniel]

Pushed bc2b1cf0. This replaces afa2adbb, whose hand-written Avro reader/writer is superseded and should not be reviewed.

• codec/wire_order.rs now reads and writes through Apache's GenericDatumReader / GenericDatumWriter, using a private binding schema derived from VALUE_SCHEMA. The schema's single map<Value> becomes array<record MapEntry{key: string, value: Value}>, the same binary layout PHP's ValueDatumWriter uses. No varint or block code remains.
• Decoding runs the public decode() first, which checks the fingerprint, schema, truncation and trailing bytes. The binding pass then reads through CompleteInput with the same truncation and trailing checks.
• Writing puts the public marker and fingerprint in front, then checks the blob again with the public decode().
• Recorded locally on the rebuilt image: published 2.11.5 passes and native passes all 19 cases. Suite results are in the PR body.

[rmcdaniel]

Pushed d85908cb, with both review findings fixed:

• Repeated map keys: wire_order.rs keeps a HashMap from key to slot. The first position and the last value are kept, without a quadratic scan.
• Registered JSON defaults now take PHP's array shape:
  • Three new raw-HTTP fixture cases. {"0":"x","1":"y"} routes as [["x","y"]]. {"0":"x","k":"y"} and {"1":"x"} crash published PHP with a 500. Native answers 500 query_default_unencodable, naming the parameter. Violations still answer first.
  • The contract pins only the 500 status for PHP's generic body.
  • Kept and documented: a default map's key order is sorted, not registered.

Recorded locally on an image built from this head: published 2.11.5 and native both pass all 22 cases. The suite results are in the PR body.

[rmcdaniel]

Re-review of exact d85908cbc974af135d2ea4bc0581b48d0221c82f clears the earlier production-source findings: the Apache-backed ordered binding, first-position/last-value slot index, PHP-shaped numeric defaults, and validation-before-encoding failure precedence match the reviewed controls. No Rust or installed Server execution was performed for this re-review.

One targeted regression-probe gap remains. scripts/conformance/server-parity/probe.php:54 loads fixtures with json_decode(..., true). query-contract-arguments-probe.php then sends those contracts unchanged through Guzzle's json option. Consequently, listdef.default in the fixture, written as {"0":"x","1":"y"}, becomes a PHP list and is sent as JSON ["x","y"]. That HTTP case does not exercise the original raw-object default bug.

I executed the exact fixture's PHP loader-to-JSON path on PHP 8.3.36: listdef is an array on the wire, while mixeddef and gapdef remain objects. A targeted object-cast control preserves listdef as {"0":"x","1":"y"}. Please preserve the intended object in the probe, assert its serialized request shape or retain an old-head counterfactual, and re-record the published/native case. The new source unit test does exercise an object; this request concerns the claimed raw-registration HTTP coverage.

The documented registered-default ordering limitations remain unfinished #325 parity work. This source review does not establish full default/cutover or published-image acceptance. The author's 22-case and library results remain attributed evidence; the reported load-sensitive execution failure is still tracked by #851.

Update for exact dbcde576aa440f437a31be204f9a16d3aea3fdd4: the targeted fixture request-shape finding is cleared by source review. A complete blob-tree comparison against d85908cb shows only the six fixture/probe/contract/test/documentation files changed; production source is unchanged. The probe casts the selected defaults to objects in the contracts passed to registration and records sent_defaults; the expected observation pins the three object encodings, and a negative contract test rejects the list encoding. The registered-default ordering limitations are now explicitly unfinished #325 work.

The author's corrected published/native recording and old-head counterfactual are retained in the qualification receipt. Those executions remain attributed; this re-review performed no Rust or installed Server execution. No targeted source-review finding remains for this slice. Normal exact-head CI and the broader unfinished parity/release requirements still apply.

[rmcdaniel]

Pushed dbcde576, which fixes the fixture request shape from review. Production source is unchanged from d85908cb.

• The probe now sends the listdef, mixeddef and gapdef defaults as JSON objects with an (object) cast. Before, the fixture loader turned {"0":"x","1":"y"} into a PHP list, which went out as a JSON array. The probe records the exact bytes it sent as sent_defaults: {"0":"x","1":"y"}, {"0":"x","k":"y"} and {"1":"x"}. The contract pins them.
• Re-recorded locally: published 2.11.5 and the native image from d85908cb both pass. listdef routes as [["x","y"]] on both, sent as an object.
• Counterfactual: with bc2b1cf0's query_contract.rs, queries.rs and wire_order.rs, a_registered_json_default_takes_phps_array_shape fails. That source routes the map blob wwHioz3/VYAiNwwCDgQCMAoCeAIxCgJ5AAA= where PHP routes the list wwHioz3/VYAiNwwCDAQKAngKAnkAAA==.
• The default key-order limits are now an open entry under "Known gaps found in review" in the port log: a named default map arrives in sorted order, and {"1":"a","0":"b"} is a list on native. They remain Replace PHP Server with a compatible Rust runtime and safe in-place upgrade #325 parity work.
• The two CI failures on d85908cb didn't involve this change. One was the composer image build: apk add brotli-libs>=1.2.1-r0 exited 5. The other was sdk-child-directions on the existing-PHP MySQL matrix, which exited 100 after its comparison had passed ({"outcome":"pass","recordings":3}). This push reruns both.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud