Skip to content

Commit 85664b2

Browse files
authored
Merge pull request #849 from durable-workflow/rust-325-query-arguments
Rust #325: route query arguments the server cannot decode
2 parents 455ee21 + 0d854ac commit 85664b2

13 files changed

Lines changed: 506 additions & 21 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ on:
6060
- tests/Unit/ServerParityConcurrentWaitsContractTest.mjs
6161
- tests/Unit/ServerParityContinueTransfersContractTest.mjs
6262
- tests/Unit/ServerParityStartSearchAttributesContractTest.mjs
63+
- tests/Unit/ServerParityQueryArgumentsContractTest.mjs
6364
- tests/Unit/ServerParityParentCloseCancellationContractTest.mjs
6465
- tests/Unit/ServerParityBridgeAdaptersContractTest.mjs
6566
- tests/Unit/ServerParityNexusOperationsContractTest.mjs
@@ -1983,7 +1984,7 @@ jobs:
19831984
bash scripts/ci/run-composer.sh --rm -v "$PWD:/source" -w /source -- \
19841985
install --working-dir=scripts/conformance/server-parity --no-interaction --no-progress
19851986
docker run --rm --user=1000:1000 --entrypoint node -v "$PWD:/source:ro" -w /source \
1986-
"$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/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
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
19871988
docker run --rm --user=1000:1000 --cpus=1 --memory=256m --memory-swap=256m \
19881989
--network=none --entrypoint node -v "$PWD:/source:ro" -w /source \
19891990
"$image" --test tests/Unit/ServerParityCodecDeadlineLoadTest.mjs > parity-evidence/codec-load-tests.txt
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# October 28, query arguments the server cannot decode
2+
3+
- **Before.** Native decoded every query's arguments at admission. It
4+
refused an undecodable blob, or one that did not decode to a list, with a
5+
bare `422 invalid_query_arguments`, whatever the run declared.
6+
- **Contract** (published 2.11.5; `WorkflowController::query`,
7+
`WorkflowQueryTaskBroker::query`, `validateContractedQueryArguments`):
8+
- The server decodes the arguments only when the run's start recorded a
9+
query contract (declared queries).
10+
- **Declared query.** After the unknown-query check, arguments that do
11+
not decode are refused before routing: `422 invalid_query_arguments`,
12+
with the message `Workflow query [<name>] arguments could not be
13+
decoded.` and `validation_errors.arguments` `["Query arguments could
14+
not be decoded."]`. Arguments that decode to a scalar rather than a PHP
15+
array are refused the same way with "must decode to an array". No query
16+
task is created.
17+
- **Named arguments (not ported here).** A string-keyed Avro map decodes
18+
to a PHP array too. With a durable query contract, Core normalizes it as
19+
named arguments (`WorkflowQueryContract::normalizeNamedArguments`), and
20+
the worker receives the positional result. Native still refuses a map as
21+
"must decode to an array". The port log records the gap (found in
22+
review).
23+
- **No declared queries.** The query is routed with its arguments as
24+
given. A query-capable worker leases a task carrying the blob unchanged,
25+
and the client receives the worker's answer.
26+
- **Fixture.** `query-arguments` (fixture key `query_argument_routing`; a
27+
`query_arguments` field already belongs to `state-queries`). Raw HTTP on
28+
protocol 1.20 in a fresh namespace:
29+
- a run whose worker declares no queries: the client query is held while
30+
the probe leases and answers the task;
31+
- a run whose worker declared the query.
32+
33+
Each run receives an undecodable blob (`AAAA`) and the avro encoding of
34+
a single string.
35+
- **Native.** Admission reads the arguments without refusing them
36+
(`queries::read_arguments`). `check_query_contract` refuses
37+
undecodable or non-list arguments only for a declared query, with PHP's
38+
message and `validation_errors`.
39+
- **Tests.** `query_argument_routing.rs` (routed to the worker; refused
40+
before routing). Both fail on the previous source.
41+
- **Also corrected.** The port log's "Start build-id pin" gap was stale:
42+
native has pinned starts since #696 (`build_pins.rs`).

‎docs/rust-server-port.md‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,20 @@ 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-
- **Undecodable query input before routing.** Native decodes query
279-
arguments at admission and refuses an undecodable blob with
280-
`invalid_query_arguments`. Published PHP first looks for a query worker,
281-
and answers `409 query_worker_unavailable` when there is none.
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.
282292
- **When an external payload reference is claimed.** Published PHP retains
283293
a referenced object once it fetches the bytes, before the command is
284294
recorded, so a refused command still retains it. Native claims it in the
@@ -449,10 +459,6 @@ contract fixture before its correction.
449459
then did not stop within the 5-second shutdown budget. Forty local
450460
repetitions of that case alone passed. Find what holds the query thread
451461
after the fence before changing either side.
452-
- **Start build-id pin.** Published PHP pins a new run to its start cohort's
453-
build id: a promoted build-id rollout, else the single build id among the
454-
live workers for the type, else among all the queue's live workers. Native
455-
starts every run unpinned, and has no build-id rollouts.
456462
- **Named-namespace routes native does not serve.** Native now serves every
457463
workflow command and operation it implements in any namespace, as PHP
458464
does ([checkpoint](rust-server-port-checkpoints/2026-10-23-named-namespace-commands.md)).
@@ -530,6 +536,22 @@ contract fixture before its correction.
530536
`start-search-attributes` fixture records typed, inferred, null, empty
531537
and refused starts against published 2.11.5
532538
([checkpoint](rust-server-port-checkpoints/2026-10-28-start-search-attributes.md)).
539+
- **Query arguments the server cannot decode** (October 28, closed).
540+
Published PHP decodes a query's arguments only to check them against a
541+
query contract the run's start recorded
542+
(`validateContractedQueryArguments`). A run that declares no queries
543+
routes the query with its arguments as given, so its worker receives even
544+
a blob that does not decode. A declared query refuses such arguments
545+
before routing, with PHP's message and `validation_errors`. Native used to
546+
refuse any undecodable blob at admission with a bare
547+
`invalid_query_arguments`. The `query-arguments` fixture records both
548+
against published 2.11.5
549+
([checkpoint](rust-server-port-checkpoints/2026-10-28-query-arguments.md)).
550+
- **Start build-id pin** (closed; entry corrected October 28). Native pins
551+
a new run to its start cohort's build id as published PHP does: a promoted
552+
build-id rollout, else the single build id among the live workers for the
553+
type, else among all the queue's live workers (`start_in`,
554+
`deployments.rs`, `build_pins.rs`). The open-gap entry predated #696.
533555
- **Trace context (Core 2.10, published in Server 2.11.1).** Core adds
534556
nullable `observability_context` columns on runs, commands and tasks.
535557
Native has no carrier: it accepts and ignores a top-level carrier on

‎rust/src/runtime/execution.rs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,13 +1124,9 @@ impl Runtime {
11241124
"query_arguments_exceed_budget",
11251125
));
11261126
}
1127-
let values = super::queries::decode(
1128-
self.signal_codec.clone(),
1129-
arguments.clone(),
1130-
true,
1131-
"invalid_query_arguments",
1132-
)
1133-
.await?;
1127+
// Read, not refused: only a declared query contract needs them
1128+
// (`validateContractedQueryArguments`).
1129+
let values = super::queries::read_arguments(self.signal_codec.clone(), arguments.clone()).await?;
11341130
match &*self.storage {
11351131
Storage::Sqlite(store) => {
11361132
store

‎rust/src/runtime/queries.rs‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,40 @@ pub(super) async fn decode(
128128
.map_err(|_| refuse(StatusCode::SERVICE_UNAVAILABLE, "query_codec_unavailable"))?
129129
}
130130

131+
/// A query's arguments as the server reads them: a list of values, or why
132+
/// they are not one. Only a declared query contract reads them
133+
/// (`validateContractedQueryArguments`); otherwise the worker receives the
134+
/// arguments as given. A map is refused as not a list, although PHP decodes
135+
/// a string-keyed map to an array and normalizes it as named arguments
136+
/// (`WorkflowQueryContract::normalizeNamedArguments`); the port's gap list
137+
/// records that.
138+
pub(super) enum QueryArguments {
139+
Decoded(Vec<crate::codec::Value>),
140+
Undecodable,
141+
NotAList,
142+
}
143+
144+
/// Decodes a query's arguments without refusing them.
145+
pub(super) async fn read_arguments(semaphore: Arc<Semaphore>, blob: String) -> Result<QueryArguments> {
146+
let permit = semaphore
147+
.acquire_owned()
148+
.await
149+
.map_err(|_| refuse(StatusCode::SERVICE_UNAVAILABLE, "query_codec_unavailable"))?;
150+
tokio::task::spawn_blocking(move || {
151+
let _permit = permit;
152+
let Ok(codec) = crate::codec::ValueCodec::new() else {
153+
return QueryArguments::Undecodable;
154+
};
155+
match codec.decode(&blob) {
156+
Ok(crate::codec::Value::Array(values)) => QueryArguments::Decoded(values),
157+
Ok(_) => QueryArguments::NotAList,
158+
Err(_) => QueryArguments::Undecodable,
159+
}
160+
})
161+
.await
162+
.map_err(|_| refuse(StatusCode::SERVICE_UNAVAILABLE, "query_codec_unavailable"))
163+
}
164+
131165
/// One admission attempt: an answer, or no worker to route the query to.
132166
pub(super) enum QueryAttempt {
133167
Answered((StatusCode, Value)),
@@ -179,14 +213,15 @@ fn admits_queries(status: &str) -> bool {
179213
}
180214

181215
/// `validateContractedQueryArguments`: a run that declares no queries leaves
182-
/// the name and arguments to the worker. A declared query without a
183-
/// parameter contract takes its arguments as given.
216+
/// the name and arguments to the worker. A declared query's arguments must
217+
/// decode to a list, and its parameter contract, when it has one, checks
218+
/// them.
184219
fn check_query_contract(
185220
contract: &Value,
186221
workflow_id: &str,
187222
run_id: &str,
188223
name: &str,
189-
values: &[crate::codec::Value],
224+
arguments: &QueryArguments,
190225
) -> Result<()> {
191226
let declared = |field: &str| contract[field].as_array().is_some_and(|items| !items.is_empty());
192227
if !declared("declared_queries") && !declared("declared_query_contracts") {
@@ -196,6 +231,22 @@ fn check_query_contract(
196231
return Err(query_refusal(StatusCode::NOT_FOUND, workflow_id, run_id, name, "rejected_unknown_query",
197232
&format!("Workflow query [{name}] is not declared on workflow [{workflow_id}].")));
198233
}
234+
// A declared query's arguments must decode to a list before routing.
235+
let (message, error) = match arguments {
236+
QueryArguments::Decoded(values) => return check_declared_arguments(contract, name, values),
237+
QueryArguments::Undecodable => ("could not be decoded", "Query arguments could not be decoded."),
238+
QueryArguments::NotAList => ("must decode to an array", "Query arguments must decode to an array."),
239+
};
240+
let mut refusal = query_refusal(StatusCode::UNPROCESSABLE_ENTITY, workflow_id, run_id, name, "invalid_query_arguments",
241+
&format!("Workflow query [{name}] arguments {message}."));
242+
if let super::RuntimeError::Protocol { response, .. } = &mut refusal {
243+
response["validation_errors"] = json!({"arguments": [error]});
244+
}
245+
Err(refusal)
246+
}
247+
248+
/// A declared query's parameter contract, when it has one.
249+
fn check_declared_arguments(contract: &Value, name: &str, values: &[crate::codec::Value]) -> Result<()> {
199250
if contract["declared_query_contracts"]
200251
.as_array()
201252
.is_some_and(|contracts| contracts.iter().any(|entry| entry["name"] == name))
@@ -298,7 +349,7 @@ where
298349
selected_run: Option<&str>,
299350
name: &str,
300351
arguments: String,
301-
values: Vec<crate::codec::Value>,
352+
values: QueryArguments,
302353
config: &QueryTaskConfig,
303354
command: Option<&Value>,
304355
) -> Result<(StatusCode, Value)> {
@@ -328,7 +379,7 @@ where
328379
selected_run: Option<&str>,
329380
name: &str,
330381
arguments: &str,
331-
values: &[crate::codec::Value],
382+
values: &QueryArguments,
332383
config: &QueryTaskConfig,
333384
command: Option<&Value>,
334385
) -> Result<QueryAttempt> {

‎rust/tests/execution.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,9 @@ mod worker_sessions;
334334
#[path = "execution/workflow_import_fences.rs"]
335335
mod workflow_import_fences;
336336

337+
#[path = "execution/query_argument_routing.rs"]
338+
mod query_argument_routing;
339+
337340
#[path = "execution/close_inventory.rs"]
338341
mod close_inventory;
339342

0 commit comments

Comments
 (0)