Skip to content

Commit dbcde57

Browse files
rmcdanielclaude
andcommitted
Rust #325: validate query arguments against their durable contract
Published Server checks a declared query's arguments with Workflow's WorkflowQueryContract::validatedArgumentsForRun: a string-keyed map is taken as named arguments, positional ones are matched in order, defaults are filled in and variadics collected, and each violation is reported with its own message under validation_errors ("argument validation failed"). The worker receives the normalized positional list, encoded again. A declared query without a parameter contract takes a list as given and refuses named arguments. Native checked positional arguments only, refused a map, reported every violation as a bare invalid_query_arguments and refused some contracts as unsupported. query_contract.rs ports the normalization and PHP's declared type rules; the query-contract-arguments fixture records twelve cases against published Server 2.11.5. Found in review of #849. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 85664b2 commit dbcde57

17 files changed

Lines changed: 1792 additions & 56 deletions

‎.github/workflows/server-parity.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ on:
6161
- tests/Unit/ServerParityContinueTransfersContractTest.mjs
6262
- tests/Unit/ServerParityStartSearchAttributesContractTest.mjs
6363
- tests/Unit/ServerParityQueryArgumentsContractTest.mjs
64+
- tests/Unit/ServerParityQueryContractArgumentsContractTest.mjs
6465
- tests/Unit/ServerParityParentCloseCancellationContractTest.mjs
6566
- tests/Unit/ServerParityBridgeAdaptersContractTest.mjs
6667
- tests/Unit/ServerParityNexusOperationsContractTest.mjs
@@ -1984,7 +1985,7 @@ jobs:
19841985
bash scripts/ci/run-composer.sh --rm -v "$PWD:/source" -w /source -- \
19851986
install --working-dir=scripts/conformance/server-parity --no-interaction --no-progress
19861987
docker run --rm --user=1000:1000 --entrypoint node -v "$PWD:/source:ro" -w /source \
1987-
"$image" --test tests/Unit/ServerParityRunnerTest.mjs tests/Unit/ServerParitySdkDirectionLanesTest.mjs tests/Unit/ServerParityCodecDeadlineCellsTest.mjs tests/Unit/ServerParityCooperativeContractTest.mjs tests/Unit/ServerParityChildCancellationContractTest.mjs tests/Unit/ServerParityScheduleContractTest.mjs tests/Unit/ServerParityVisibilityContractTest.mjs tests/Unit/ServerParityAdmissionContractTest.mjs tests/Unit/ServerParityRoleTokenContractTest.mjs tests/Unit/ServerParityWorkerDeregistrationContractTest.mjs tests/Unit/ServerParitySearchAttributeContractTest.mjs tests/Unit/ServerParityWorkerListingContractTest.mjs tests/Unit/ServerParityWorkerReplacementContractTest.mjs tests/Unit/ServerParityPatchDeploymentContractTest.mjs tests/Unit/ServerParityWaitingHistoryContractTest.mjs tests/Unit/ServerParityTaskRetryContractTest.mjs tests/Unit/ServerParityPublishedPatchContractTest.mjs tests/Unit/ServerParityPublishedPatchTransportTest.mjs tests/Unit/ServerParityRecordFileTest.mjs tests/Unit/ServerParityFailureLocationTest.mjs tests/Unit/ServerParityFailureExcerptTest.mjs tests/Unit/ServerParityLateWorkflowCompletionContractTest.mjs tests/Unit/ServerParityStorageTestContractTest.mjs tests/Unit/ServerParitySchedulePoliciesContractTest.mjs tests/Unit/ServerParitySignalsBeforeWorkerContractTest.mjs tests/Unit/ServerParityScheduleCatalogContractTest.mjs tests/Unit/ServerParityOperatorMetricsContractTest.mjs tests/Unit/ServerParityActivityLeaseRenewalContractTest.mjs tests/Unit/ServerParityConcurrentWaitsContractTest.mjs tests/Unit/ServerParityContinueTransfersContractTest.mjs tests/Unit/ServerParityStartSearchAttributesContractTest.mjs tests/Unit/ServerParityQueryArgumentsContractTest.mjs tests/Unit/ServerParityParentCloseCancellationContractTest.mjs tests/Unit/ServerParityRecordLocalActivityContractTest.mjs tests/Unit/ServerParityActivityCancellationAckContractTest.mjs tests/Unit/ServerParityServiceCatalogContractTest.mjs tests/Unit/ServerParityServiceEndpointsContractTest.mjs tests/Unit/ServerParityServiceCallsContractTest.mjs tests/Unit/ServerParityWorkflowImportContractTest.mjs tests/Unit/ServerParityWaterlineImportContractTest.mjs tests/Unit/ServerParitySystemMetricsContractTest.mjs tests/Unit/ServerParityOperatorDashboardTypesContractTest.mjs tests/Unit/ServerParityPrometheusMetricsContractTest.mjs tests/Unit/ServerParityPayloadCleanupContractTest.mjs tests/Unit/ServerParityRepairPassContractTest.mjs tests/Unit/ServerParityClusterInfoContractTest.mjs tests/Unit/ServerParityNamespaceDeleteContractTest.mjs tests/Unit/ServerParityMessageStreamsContractTest.mjs tests/Unit/ServerParityCancellationScopesContractTest.mjs tests/Unit/ServerParityWorkflowPollRequestContractTest.mjs tests/Unit/ServerParityUpdateValidationContractTest.mjs tests/Unit/ServerParityFixtureSelectionTest.mjs tests/Unit/ServerParityUpdateValidationRetryTest.mjs tests/Unit/ServerParityKilledAuthorPollTest.mjs tests/Unit/ServerParityImagePullTest.mjs tests/Unit/ServerParityAdapterCacheTest.mjs tests/Unit/ServerParityWorkerSessionsContractTest.mjs tests/Unit/ServerParityWorkerSessionActivitiesContractTest.mjs tests/Unit/ServerParityNexusOperationsContractTest.mjs tests/Unit/ServerParityBridgeAdaptersContractTest.mjs tests/Unit/ServerParityWorkflowStreamWritesContractTest.mjs > parity-evidence/comparator-tests.txt
1988+
"$image" --test tests/Unit/ServerParityRunnerTest.mjs tests/Unit/ServerParitySdkDirectionLanesTest.mjs tests/Unit/ServerParityCodecDeadlineCellsTest.mjs tests/Unit/ServerParityCooperativeContractTest.mjs tests/Unit/ServerParityChildCancellationContractTest.mjs tests/Unit/ServerParityScheduleContractTest.mjs tests/Unit/ServerParityVisibilityContractTest.mjs tests/Unit/ServerParityAdmissionContractTest.mjs tests/Unit/ServerParityRoleTokenContractTest.mjs tests/Unit/ServerParityWorkerDeregistrationContractTest.mjs tests/Unit/ServerParitySearchAttributeContractTest.mjs tests/Unit/ServerParityWorkerListingContractTest.mjs tests/Unit/ServerParityWorkerReplacementContractTest.mjs tests/Unit/ServerParityPatchDeploymentContractTest.mjs tests/Unit/ServerParityWaitingHistoryContractTest.mjs tests/Unit/ServerParityTaskRetryContractTest.mjs tests/Unit/ServerParityPublishedPatchContractTest.mjs tests/Unit/ServerParityPublishedPatchTransportTest.mjs tests/Unit/ServerParityRecordFileTest.mjs tests/Unit/ServerParityFailureLocationTest.mjs tests/Unit/ServerParityFailureExcerptTest.mjs tests/Unit/ServerParityLateWorkflowCompletionContractTest.mjs tests/Unit/ServerParityStorageTestContractTest.mjs tests/Unit/ServerParitySchedulePoliciesContractTest.mjs tests/Unit/ServerParitySignalsBeforeWorkerContractTest.mjs tests/Unit/ServerParityScheduleCatalogContractTest.mjs tests/Unit/ServerParityOperatorMetricsContractTest.mjs tests/Unit/ServerParityActivityLeaseRenewalContractTest.mjs tests/Unit/ServerParityConcurrentWaitsContractTest.mjs tests/Unit/ServerParityContinueTransfersContractTest.mjs tests/Unit/ServerParityStartSearchAttributesContractTest.mjs tests/Unit/ServerParityQueryArgumentsContractTest.mjs tests/Unit/ServerParityQueryContractArgumentsContractTest.mjs tests/Unit/ServerParityParentCloseCancellationContractTest.mjs tests/Unit/ServerParityRecordLocalActivityContractTest.mjs tests/Unit/ServerParityActivityCancellationAckContractTest.mjs tests/Unit/ServerParityServiceCatalogContractTest.mjs tests/Unit/ServerParityServiceEndpointsContractTest.mjs tests/Unit/ServerParityServiceCallsContractTest.mjs tests/Unit/ServerParityWorkflowImportContractTest.mjs tests/Unit/ServerParityWaterlineImportContractTest.mjs tests/Unit/ServerParitySystemMetricsContractTest.mjs tests/Unit/ServerParityOperatorDashboardTypesContractTest.mjs tests/Unit/ServerParityPrometheusMetricsContractTest.mjs tests/Unit/ServerParityPayloadCleanupContractTest.mjs tests/Unit/ServerParityRepairPassContractTest.mjs tests/Unit/ServerParityClusterInfoContractTest.mjs tests/Unit/ServerParityNamespaceDeleteContractTest.mjs tests/Unit/ServerParityMessageStreamsContractTest.mjs tests/Unit/ServerParityCancellationScopesContractTest.mjs tests/Unit/ServerParityWorkflowPollRequestContractTest.mjs tests/Unit/ServerParityUpdateValidationContractTest.mjs tests/Unit/ServerParityFixtureSelectionTest.mjs tests/Unit/ServerParityUpdateValidationRetryTest.mjs tests/Unit/ServerParityKilledAuthorPollTest.mjs tests/Unit/ServerParityImagePullTest.mjs tests/Unit/ServerParityAdapterCacheTest.mjs tests/Unit/ServerParityWorkerSessionsContractTest.mjs tests/Unit/ServerParityWorkerSessionActivitiesContractTest.mjs tests/Unit/ServerParityNexusOperationsContractTest.mjs tests/Unit/ServerParityBridgeAdaptersContractTest.mjs tests/Unit/ServerParityWorkflowStreamWritesContractTest.mjs > parity-evidence/comparator-tests.txt
19881989
docker run --rm --user=1000:1000 --cpus=1 --memory=256m --memory-swap=256m \
19891990
--network=none --entrypoint node -v "$PWD:/source:ro" -w /source \
19901991
"$image" --test tests/Unit/ServerParityCodecDeadlineLoadTest.mjs > parity-evidence/codec-load-tests.txt

‎docs/rust-server-port-checkpoints/2026-10-28-query-arguments.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
named arguments (`WorkflowQueryContract::normalizeNamedArguments`), and
2020
the worker receives the positional result. Native still refuses a map as
2121
"must decode to an array". The port log records the gap (found in
22-
review).
22+
review); the query-contract-arguments slice closes it.
2323
- **No declared queries.** The query is routed with its arguments as
2424
given. A query-capable worker leases a task carrying the blob unchanged,
2525
and the client receives the worker's answer.
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# October 28, named and validated query arguments
2+
3+
- **Before.** For a declared query with a parameter contract, native
4+
differed from published PHP in four ways:
5+
- it checked positional arguments only;
6+
- it refused a named-argument map as "must decode to an array";
7+
- it answered every violation with a bare `invalid_query_arguments`;
8+
- it refused some contracts as `unsupported_query_contract` (defaults,
9+
and declared types other than a few scalars).
10+
11+
The worker then received the arguments exactly as the client sent them.
12+
- **Contract** (published 2.11.5, Core 2.15.1;
13+
`WorkflowQueryTaskBroker::validateContractedQueryArguments`,
14+
`WorkflowQueryContract::validatedArgumentsForRun`,
15+
`normalizePositionalArguments`, `normalizeNamedArguments`,
16+
`valueMatchesDeclaredType`):
17+
- **Arguments.** An Avro array is a positional list. A non-empty map
18+
with no key PHP casts to an integer is a PHP array of named arguments.
19+
`Avro::fromDatum` returns any other map (empty, or with a canonical
20+
decimal integer key) as an `AvroMapValue` object, which is not an array.
21+
At the top level it is refused before normalization with "must decode to
22+
an array" (found in review).
23+
- **Positional.** Values fill the parameters in order. A variadic
24+
parameter takes the rest. A missing argument takes its default or is
25+
reported if required. Extra values are reported under `arguments`.
26+
- **Named.** Each parameter takes its name's value. A variadic parameter
27+
spreads a list or a map's values. Defaults and required arguments are
28+
handled as for positional ones. Each unknown name is reported under
29+
itself.
30+
- **Values.** A null is refused unless the parameter allows it. A value
31+
must match the declared type: nullable, union, intersection, `int`,
32+
`float` (an int too), `string`, `bool`, `array`, `iterable`, `scalar`,
33+
`true`, `false`, `null`, `mixed`. An adapter map is not an `array`. A
34+
class or `object` type matches only an object: decoded bytes or an
35+
adapter map. Neither object is `callable`. A named variadic spreads a
36+
PHP array's values in wire order and keeps any other value, an adapter
37+
map included, as one argument (found in review).
38+
- **Refusal.** Any violation answers `422 invalid_query_arguments`, with
39+
the message "Workflow query [<name>] argument validation failed." and
40+
`validation_errors` by argument.
41+
- **Worker.** Otherwise the worker receives the normalized positional
42+
list, encoded again with the request's codec.
43+
- **No parameter contract.** A declared query without one takes a list as
44+
given and refuses named arguments: "Named arguments require a durable
45+
or loadable workflow query contract." (a native server never loads a
46+
workflow class).
47+
- **Fixture.** `query-contract-arguments`, raw HTTP on protocol 1.20 in a
48+
fresh namespace. The worker declares `state(string $prefix)`,
49+
`greet(string $prefix = 'dflt')`, `plain`, `arr(array $value)`,
50+
`obj(object $value)` and `rest(mixed ...$value)`, and three `mixed`
51+
queries whose registered defaults are raw JSON objects. There are
52+
twenty-two cases:
53+
- named, positional, default and named-default queries are routed, and
54+
the worker's blob is checked;
55+
- unknown, missing, wrong-type, null, too-many and named-without-contract
56+
queries are refused, with their messages checked;
57+
- an empty map, a `{"0": "ok"}` map and a mixed-key map are refused as not
58+
arrays;
59+
- a nested empty map is refused by `array`, but routed whole by `object`
60+
and by the variadic `mixed`;
61+
- `{value: {z: 20, a: 10}}` reaches the variadic `rest` as `[20, 10]`;
62+
- a registered default `{"0": "x", "1": "y"}`, sent as a JSON object
63+
(the probe records the bytes it sends), reaches the worker as the list
64+
`[["x", "y"]]`. On the previous source native routed it as a map;
65+
- a registered default `{"0": "x", "k": "y"}` or `{"1": "x"}` fails
66+
published PHP's Avro encoder (`invalid_map_key`) with an uncaught 500.
67+
The fixture pins only that status.
68+
- **Native.** `query_contract.rs` ports the normalization and type rules
69+
over `codec::OrderedValue`. Apache's generic reader keeps maps unordered,
70+
so the new `codec/wire_order.rs` reads the bytes the public reader has
71+
already validated again through a private binding schema. That schema
72+
spells each map as an array of key/value records, as PHP's
73+
`ValueDatumWriter` does, so the binary layout is the same and Apache still
74+
reads every primitive and block. Writing goes through the same binding,
75+
under the public header, and the public reader checks the result. For every
76+
fixture blob the bytes match PHP's.
77+
A registered default takes PHP's array shape: canonical integer keys
78+
0 to n-1 make a list, string keys a map, and any other integer key is
79+
answered 500 `query_default_unencodable`, naming the parameter.
80+
Violations answer first, as PHP encodes only valid arguments.
81+
`check_query_contract` returns the normalized list for a declared query,
82+
or the refusal. `query_attempt` enqueues the re-encoded blob.
83+
- **Kept.**
84+
- A registered default map keeps its keys in sorted order, not the
85+
registered order: native's JSON objects do not preserve order
86+
(`serde_json` without `preserve_order`). The same limit makes
87+
`{"1": "a", "0": "b"}` a list on native, where PHP's array is not a list
88+
and fails to encode.
89+
- `callable` matches nothing here: PHP's `is_callable` also accepts
90+
strings naming functions, which a native server cannot resolve.
91+
- A class name that a published PHP server can load (a real class) would
92+
refuse other objects. Native, loading no classes, accepts any object.
93+
- **Tests.** `query_contract_arguments.rs` (routed normalization; refusals
94+
with messages) fails on the previous source. `query_contract.rs` unit
95+
tests cover the type rules and integer-keyed maps.

‎docs/rust-server-port.md‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -275,20 +275,6 @@ contract fixture before its correction.
275275
each priority tier across fairness keys by their weights
276276
(`TaskFairnessScheduler`); native stores the keys and weights but does not
277277
rebalance yet.
278-
- **Named and validated query arguments.** For a declared query with a
279-
durable parameter contract, published PHP normalizes the arguments through
280-
`WorkflowQueryContract::validatedArgumentsForRun`, then gives the worker the
281-
re-encoded positional result:
282-
- a string-keyed map is taken as named arguments;
283-
- positional arguments are matched to the parameters;
284-
- defaults are filled in, variadics collected, and unknown names
285-
reported;
286-
- a violation answers `422 invalid_query_arguments`, "argument validation
287-
failed", with per-parameter messages in `validation_errors`.
288-
289-
Native checks positional arguments only. It refuses a map as "must decode
290-
to an array", reports a violation as a bare `invalid_query_arguments`, and
291-
gives the worker the arguments as sent.
292278
- **When an external payload reference is claimed.** Published PHP retains
293279
a referenced object once it fetches the bytes, before the command is
294280
recorded, so a refused command still retains it. Native claims it in the
@@ -552,6 +538,29 @@ contract fixture before its correction.
552538
build-id rollout, else the single build id among the live workers for the
553539
type, else among all the queue's live workers (`start_in`,
554540
`deployments.rs`, `build_pins.rs`). The open-gap entry predated #696.
541+
- **Named and validated query arguments** (October 28, closed). For a
542+
declared query, published PHP matches the arguments to the query's durable
543+
parameter contract (`WorkflowQueryContract::validatedArgumentsForRun`):
544+
- a string-keyed map is taken as named arguments;
545+
- positional arguments are matched in order;
546+
- defaults are filled in and variadics collected;
547+
- unknown names, missing required arguments, nulls, wrong types and extra
548+
arguments are each reported. Declared types may be nullable, unions,
549+
intersections, scalars, `array`, `object` or class names.
550+
551+
A violation answers `422 invalid_query_arguments`, "argument validation
552+
failed", with per-argument messages in `validation_errors`. The worker
553+
receives the normalized positional list, encoded again. A declared query
554+
without a parameter contract takes a list as given and refuses named
555+
arguments. Native now does the same (`query_contract.rs`). It used to
556+
check positional arguments only, refuse a map, report a violation as a
557+
bare `invalid_query_arguments`, and refuse some contracts as unsupported.
558+
`Avro::fromDatum` returns an empty or integer-keyed map as an object, not
559+
an array, and a named variadic map spreads in wire order. A registered
560+
JSON default takes PHP's array shape. The `query-contract-arguments`
561+
fixture records twenty-two cases against
562+
published 2.11.5
563+
([checkpoint](rust-server-port-checkpoints/2026-10-28-query-contract-arguments.md)).
555564
- **Trace context (Core 2.10, published in Server 2.11.1).** Core adds
556565
nullable `observability_context` columns on runs, commands and tasks.
557566
Native has no carrier: it accepts and ignores a top-level carrier on
@@ -647,6 +656,18 @@ contract fixture before its correction.
647656
- **Command route names.** Published PHP records each request's route name in
648657
its command context, and its unnamed workflow routes get a name generated
649658
per deployment. Native records names only for the named schedule routes.
659+
- **Registered default key order.** A worker registers query parameter
660+
defaults as JSON. PHP keeps each object's keys in the registered order,
661+
and a default map reaches the worker in that order. Native's JSON objects
662+
keep keys sorted (`serde_json` without `preserve_order`), so:
663+
- a named default map such as `{"z": 20, "a": 10}` reaches the worker in
664+
sorted order;
665+
- `{"1": "a", "0": "b"}` becomes a list on native, where PHP's array is
666+
not a list and its encoder fails with a 500.
667+
668+
Both need registered-order JSON in native contracts and a fixture case
669+
each; this remains open #325 parity work
670+
([checkpoint](rust-server-port-checkpoints/2026-10-28-query-contract-arguments.md)).
650671

651672
## Contract and verification inventory
652673

‎rust/src/codec.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ use std::collections::BTreeMap;
66
use std::io::{self, Read};
77
mod ordered;
88
mod validation;
9+
mod wire_order;
10+
pub use wire_order::OrderedValue;
911

1012
use apache_avro::{
1113
GenericSingleObjectReader, GenericSingleObjectWriter, Schema, types::Value as Datum,

0 commit comments

Comments
 (0)