fix: Send the action attempt poll id as a query - #467
Merged
Conversation
The resolver polled /action_attempts/get with the id in a JSON body on a GET, while the generated route for the same endpoint sends it as a query. A GET body is not carried reliably: any proxy, CDN, or load balancer that strips one breaks every wait loop, and it breaks it after the write has already been commanded. The body also skipped the serializing client's query handling, so _strict=true was never applied to the poll. Send a query instead, matching the generated route. The fake server reads GET bodies, so the wire shape is asserted with the recording client. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HH3wdHh4Y6Wjyc5uHwk5iG
razor-x
force-pushed
the
claude/php-audit-uoa7nb-m1-poll-query
branch
from
August 19, 2026 21:44
313e5e6 to
c35ffd6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes finding M1 from the cross-SDK audit.
Rebased onto
betaata2cd8ce(4.0.0-beta.13), so the conflict is gone. Re-checked against beta after #478 changed HTTP verbs: the generated/action_attempts/getroute is stillGETwith a query, so the mismatch this fixes is still real.The problem
ResolveActionAttempt::get_action_attemptpolls with the id in a JSON body on aGET:The generated route for the same endpoint sends a query (
src/Routes/ActionAttemptsClient.php):Two consequences:
_strict=truewas never applied to the poll.SerializingClientonly adds it to a query, so the poll skipped the schema-aware parsing every other read gets.The fake server reads GET bodies happily, which is why no existing test caught it.
The fix
Send a query, matching the generated route. The poll now goes out as
GET /action_attempts/get?action_attempt_id=…&_strict=truewith an empty body.Tests
testPollSendsTheIdAsAQueryNotABodyasserts the poll's method, path, query string, and empty body viaRecordingClient— the fake can't show this. Against currentbetait fails with the query string empty:Full suite: 229 tests green, psalm clean,
npm run lintclean. The deprecations in the run are the resource-constructor issue from #473, not yet onbeta.Note on scope
The audit's preferred form is polling through the generated
ActionAttemptsClientwithwait_for_action_attempt: false, as the JS SDK does. That requires threading the route client through codegen and touches every generated route; this PR fixes the wire shape only. Happy to follow up with the injection version if you'd rather have it.Generated by Claude Code