Skip to content

fix(rest/python): complete webhook-test checkouts by server-assigned id - #180

Open
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:fix/webhook-tests-server-assigned-id
Open

fix(rest/python): complete webhook-test checkouts by server-assigned id#180
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:fix/webhook-tests-server-assigned-id

Conversation

@vishkaty

Copy link
Copy Markdown
Contributor

What broke

The Python Server Tests job is red on main since #169 merged: every webhook signing test fails with 404 RESOURCE_NOT_FOUND at the complete step.

Root cause: an interaction between two individually-green changes, both mine. #169's _completed_checkout helper (written before #167 landed) creates a checkout and then completes it using the client-supplied id. #167 makes the server assign checkout ids, so on the merged tree the complete call targets an id that does not exist. Each PR passed CI when last run because the fork test job runs at review time — the combination only ever executed after both merged.

Fix

_completed_checkout reads the server-assigned id from the create response and completes against it (the same adjustment #167 applied to the other integration tests).

Verification

  • On main tip ab78116: reproduced all webhook-test failures locally with the same 404.
  • With this fix: webhook tests 11/11, full suite 164 passed, 0 failed, pinned pre-commit clean.

Apologies for the breakage — both halves were mine, and the lesson (re-run the suite against the merged tree when sibling PRs land between approval and merge) is taken.

The webhook signing tests from Universal-Commerce-Protocol#169 drive create + complete through
_completed_checkout using the client-supplied checkout id. Universal-Commerce-Protocol#167 made the
server assign checkout ids, so on a tree containing both changes the
complete call answers 404 RESOURCE_NOT_FOUND and every webhook test
fails — the Python Server Tests job is red on main since Universal-Commerce-Protocol#169 merged
(the two changes were each green when last exercised by CI, and the
interaction only exists on the merged tree).

Read the server-assigned id from the create response and complete
against it. Full suite: 164 passed.
@damaz91 damaz91 added status:needs-triage Signal that the PR is ready for human triage status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants