Skip to content

Rust #325: route query arguments the server cannot decode - #849

Merged
rmcdaniel merged 1 commit into
mainfrom
rust-325-query-arguments
Oct 29, 2026
Merged

rmcdaniel merged 1 commit into
mainfrom
rust-325-query-arguments

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Part of #325.

What changes

Published Server decodes a query's arguments only to check them against a query contract the run's start recorded (WorkflowQueryTaskBroker::query, validateContractedQueryArguments):

  • No declared queries. The query is routed with its arguments as given. A query-capable worker leases a task carrying even a blob that does not decode, or one that decodes to a single value, unchanged, and the client receives the worker's answer.
  • Declared query. After the unknown-query check, arguments that do not decode are refused before routing with 422 invalid_query_arguments, the message Workflow query [<name>] arguments could not be decoded. and validation_errors.arguments: ["Query arguments could not be decoded."]. Arguments that decode to a scalar rather than a PHP array get the "must decode to an array" variant. No query task is created.

Not ported here (recorded in the port log): a string-keyed map is also a PHP array. With a durable query contract, Core normalizes it as named arguments (WorkflowQueryContract::normalizeNamedArguments), validates each parameter with its own messages, and re-encodes the positional result for the worker. Native still refuses a map, and reports contract violations as a bare invalid_query_arguments. The next PR ports WorkflowQueryContract.

Native decoded every query's arguments at admission and refused an undecodable or non-list blob with a bare invalid_query_arguments, whatever the run declared.

Native now:

  • reads the arguments without refusing them at admission (queries::read_arguments, QueryArguments);
  • refuses them in check_query_contract, only for a declared query, with PHP's message and validation_errors.

The port log's "Start build-id pin" entry was stale: native has pinned starts since #696 (build_pins.rs). It is corrected here.

Contract

The new query-arguments fixture runs over raw HTTP on protocol 1.20 in a fresh namespace. Its fixture key is query_argument_routing, because a query_arguments field already belongs to state-queries. It records against published 2.11.5:

  • A run whose worker declares no queries. The client query is held while the probe leases the query task, records the arguments it carries, and answers it.
  • A run whose worker declared the query.

Each run receives an undecodable blob (AAAA) and the avro encoding of a single string. Embedded applications have no worker protocol, so the case is explicitly inapplicable there. The probe retries lock-pressure refusals like the other probes. Contract: query-arguments-contract.mjs. Its unit test is registered in server-parity.yml.

Verification (run locally on the Rust dev host)

  • Fixture: recorded against published 2.11.5 on fresh SQLite state, and against a native image built from this branch. Both pass.

  • Native tests (SQLite, after rebasing onto Rust #325: store search attributes named on a workflow start #848, in this branch's own cargo target directory): cargo test --test execution 621 passed, cargo test --lib 347 passed.

  • New tests in query_argument_routing.rs:

    • routed to the worker unchanged;
    • refused before routing, with message and validation_errors.

    Both fail on the previous source. The existing query tests pass (32).

  • Node: the contract, runner and fixture-selection unit tests pass (297).

  • Not run locally: PostgreSQL and MySQL; CI runs them.

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

🤖 Generated with Claude Code

@rmcdaniel

rmcdaniel commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

Source review of exact 0f80dadfc89b09cd2996a2498ccb319b77baed68: the undecodable and scalar-string cases match published Server 2.11.5’s broker ordering. Terminal status and unknown declared query checks precede argument refusal; an uncontracted run retains the original blob for worker routing. The new fixture checks the original bytes and distinguishes the two refusal messages.

One existing boundary should remain explicit: PHP’s “array” also permits named arguments. Server 2.11.5 locks Core 2.15.1 (07e0d0e1a1185bd0a3bbd4e3fb56511cc57ca9bc). Its Avro::fromDatum returns nonempty string-key maps as associative arrays, and WorkflowQueryContract::validatedArgumentsForRun normalizes them through normalizeNamedArguments when a durable query contract is present. Native read_arguments classifies every Value::Map as NotAList and refuses it for a declared query.

A concrete additional case is a declared state(prefix: string) query given an Avro map {"prefix":"ok"}: published PHP should normalize it to positional ["ok"] for the worker. The current fixture’s non-list case is a string and does not cover this. Please verify and retain named-query normalization as a remaining gap, or cover it here. The strict-list restriction predates this delta; this is not a claim of a newly introduced regression.

This review covers source and fixture expectations. I did not execute the Rust or HTTP tests.

Follow-up on exact 0d854ac16d281e587931af29781c543a5317006d: the PR description, checkpoint and port log now explicitly retain the named/validated query-argument gap, including defaults, variadics, parameter messages and positional re-encoding. Exact query-source comparison shows a comment-only change from the reviewed head, and execution.rs is byte-identical. This resolves the scope/documentation concern; named-argument parity remains unfinished work under #325. Hosted checks remain the gate for this PR.

Hosted qualification for exact 0d854ac16d281e587931af29781c543a5317006d is terminal at 2026-10-28 23:59 UTC: 56 successful checks, one skipped check, zero pending and zero failed checks. This completes the hosted gate for the reviewed head; named/validated query-argument parity remains recorded separately under #325.

Published Server decodes a query's arguments only to check them against
a query contract the run's start recorded (validateContractedQueryArguments).
A run that declares no queries routes the query with its arguments as
given, so its worker receives even a blob that does not decode; a declared
query refuses such arguments before routing, with PHP's message and
validation_errors. Native refused any undecodable blob at admission with
a bare invalid_query_arguments.

The query-arguments fixture records both against published 2.11.5. The
port log's stale "Start build-id pin" gap is corrected: native has pinned
starts since #696.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel
rmcdaniel force-pushed the rust-325-query-arguments branch from 0f80dad to 0d854ac Compare October 28, 2026 23:33
@rmcdaniel

Copy link
Copy Markdown
Member Author

Thanks for the named-argument review. You're right: a string-keyed Avro map decodes to a PHP array, and with a durable query contract Core normalizes it as named arguments (WorkflowQueryContract::normalizeNamedArguments). Native still refuses a map. That restriction predates this PR, as you noted.

At 0d854ac1:

  • Docs: the checkpoint, the PR description and the read_arguments comment no longer imply that every PHP array is a list. The "must decode to an array" refusal is described as the scalar case.
  • Gap recorded: the port log has a new open entry, "Named and validated query arguments". It covers named maps, positional normalization with defaults and variadics, per-parameter validation messages, and the re-encoded positional arguments the worker receives.

No runtime source changed. I'll port WorkflowQueryContract::validatedArgumentsForRun in the next PR, with a fixture covering your state(prefix: string) with {"prefix":"ok"} case.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 85664b2 Oct 29, 2026
57 checks passed
@rmcdaniel
rmcdaniel deleted the rust-325-query-arguments branch October 29, 2026 00:22
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
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>
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
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>
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
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>
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
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>
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
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>
rmcdaniel added a commit that referenced this pull request Oct 29, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant