Repository navigation
fix: correct the product feed contents and the discovery document - #16
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ore serves Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
✅ Magebit Code Review — Review completeNo issues met the confidence threshold to publish. Changed files (18)Api/
Model/
Test/
etc/
(root)
Reviewed by |
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.
Seven defects in the product feed and the discovery document, all found by running an export
against a real catalogue and by validating every response against the vendored ACP JSON Schema.
The export crashed, and the file it wrote was unusable
magebit:agentic-commerce:exportdied withConfig::getSellerName(): Return value must be of type string, null returned. The specificationmakes the seller name optional, so the return type is now nullable. Two
@phpstan-ignore return.typecomments had been hiding this.Fixing the crash let the export finish, and the file it produced was wrong: the header had 22
columns while 1847 of 1891 rows had 27, so almost every row was written shifted — a variant's price
landed under an option label. Three causes:
order. Products do not all carry the same columns, because a variant adds a pair per option.
Rows are now buffered while the full column set is learned, then written against one header.
mid-row and the work was repeated once per column.
formatValuethrew away the value it was given and read it again without the parentproduct. Every column sourced from the parent came out empty, so all 1847 variants had no
item_group_idand an agent could not group them.A fifth defect showed up while checking the output: the bundle product was advertised at
0.00 USD with checkout enabled, when it really costs 61 to 77. Bundle and grouped products hold
no price of their own, and the
priceattribute is the string'0.000000'— truthy, so theformatter's guard never fired. A new
Source\Pricefalls back to the indexed minimum.After the fixes: 27 columns, 1891 rows, every row 27 fields wide, all 1847 variants carrying
item_group_id, no blank ids, no blank or zero prices, no blank seller names.The discovery document did not match the schema
ACP publishes no conformance suite, so nothing had ever checked our reading of the specification
from outside. A new harness drives every operation the vendored specification defines over HTTP and
validates each response against the specification's own JSON Schema. It found three divergences:
api_base_urlnamed the checkout sessions rather than what they hang off, so an agent followingthe schema's own instruction requested
/checkout_sessions/checkout_sessions.USD; the schema asks for lowercase.en_USrather than the BCP 47en-US.Two existing unit tests had been asserting the wrong values, which is how all three survived. The
document also now advertises the services the store actually serves.
Tests
20 new tests, 189 passing in the module. Each was checked by putting the defect back and confirming
the test fails.
phpcs,phpstanand the unit suite are green; six now-stale entries were removedfrom the PHPStan baseline, which had been locking several of these bugs in.
🤖 Generated with Claude Code