index: fold the manifest catalogue in without duplicating or losing variants - #658
Merged
Conversation
…er as extra rows Regression fix for the previous commit, caught on the live index within an hour of deploy. Reading a seller's manifest catalogue was supposed to take their listing from 16 to 17. It took it to 30. Manifest entries declare no HTTP verb, so they default to GET, and they often carry a query template. The same endpoint arrives from a registry row or an openapi operation as a bare POST path. Neither the method nor the route string matches, so the route-keyed merge saw two endpoints where there was one: POST /x402/preflight (registry row) GET /x402/preflight?chain=base&sender=... (manifest entry) Eleven of that seller's seventeen entries doubled exactly this way. Listing one endpoint twice overstates the seller and hands the router two candidates that are one, which is the inflation failure this file has had to undo before, and it is worse than the thin listing the read was meant to fix. mergeManifestIntoTools now folds the catalogue in LAST and keys on the PATHNAME alone. An existing row wins on shape, because a verb from an openapi operation or a settled registry row is observed while a manifest's silence is not. The manifest wins on description: name, summary and price fill gaps the existing row left, which is the whole reason to read it. A claimed value never overwrites an observed one. Collapsing applies only against PRE-EXISTING rows. A seller whose manifest is the sole source for a path keeps their query-distinct products, since ?product=commodities and ?product=market_brief are two offerings at two prices; the moment another source reports that path, both fold into it. Replayed against the real manifest and the rows the live index holds: naive concat 25, pathname merge 16. 8 assertions added (43 total). Keying the merge on the full route reproduces the regression and kills 5; letting a claimed value overwrite an observed one kills 2; dropping manifest-only endpoints returns to thin listings and kills 4. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…not duplicating them
Follow-up to the previous commit, prompted by the seller who reported the
original issue reviewing the fix and finding what it cost them.
Keying the merge on pathname stopped the 16 to 30 duplication, and it silently
lost variants. A single route often sells different things by parameter, at
different prices: ?product= here, but equally a reader keyed by ?url= or a
chain call keyed by ?chain=. Folding those into one row erases products the
seller does sell, and the manifest carries the full resource string verbatim.
Keying on that full string instead is not the answer either; that is exactly
what produced the duplication, because the same endpoint arrives from a
registry row as a bare POST path and from the manifest as a GET with a query
template, and neither method nor route matches.
So the pathname decides the MATCH and the number of resources the seller
advertises on that path decides the OUTCOME:
path unknown to us -> add everything, variants included
one resource, path known -> same endpoint; enrich in place, add nothing
several resources, path known -> the row we hold is that path without its
parameters, so the variants replace it
Replacing rather than sitting beside is the point of the third case: keeping
both would list one endpoint N+1 times, which is the duplication this function
exists to prevent. The observed verb carries across to the variants, since a
manifest declares none and a defaulted GET must never overwrite a POST we
actually saw.
Replayed against the real manifest and the rows the live index holds: 17
entries, both parameter variants preserved, zero paths listed twice.
47 assertions total. All three failure modes are covered and mutation-checked:
keying on the full route reproduces the duplication (kills 8), folding
variants into one row reproduces the loss (kills 5), and keeping the bare row
beside the variants lists it N+1 times (kills 5).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Follow-up to #657, which shipped the manifest-catalogue read and two defects with it. Both were found within the hour, one on our own index and one by the seller who filed #645 reviewing the patch.
The two defects
Duplication (16 to 30). The merge keyed on method plus route. Manifest entries declare no HTTP verb, so they default to GET and carry query templates, while the same endpoints arrive from a registry as bare POST paths. Nothing matched, so eleven of one seller's seventeen entries were listed twice. Listing one endpoint twice overstates the seller and hands the router two candidates that are one.
Variant loss. Keying on the pathname alone fixes the duplication and silently folds away real products. A single route often sells different things by parameter, at different prices:
?product=here, but equally a reader keyed by?url=or a chain call keyed by?chain=.Keying on the full advertised resource string is not the answer to the second, because it is the cause of the first.
The rule
The pathname decides the MATCH; how many resources the seller advertises on that path decides the OUTCOME.
Replacing rather than sitting beside is the point of the third case: keeping both lists one endpoint N+1 times. An observed verb always beats a manifest's silence, since a verb from an openapi operation or a settled registry row is evidence and a default is not. The manifest still wins on description, which is the reason to read it.
Verification
scripts/test-discovery-note.js, in CI