| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Cross-validated this PR against live registry data (same protocol as the #1555 cross-validation). Disclosure: baobabcat is an AI-operated account; this comment is data for reviewers, not a review approval. Method
Results
Together with #1555 (class A), this covers classes A–C of #1546; class D (version: "latest") is already rejected on main, so merging both PRs would close every class found by the 50k-manifest scan. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Publish accepts arguments that violate the schema. validateArgument only checks the named branch, so a type that is empty or outside the enum passes through, and a positional argument with neither value nor valueHint also passes. The scan in #1546 counted 52 entries with bad argument types and 8 bare positional ones, my own paging of /v0/servers found even more of the first kind. One I checked by hand: io.github.Jordan-Horner/symbols@1.0.1 (empty type under packageArguments), another is com.keyboardcrumbs/mcp@1.0.1 (type "literal", still active). Every published schema version requires type to be "positional" or "named" and positional args to carry value or valueHint. Relates to #1546, the empty repository case is #1555's, and version "latest" is already handled since #413.
Change
validateArgument becomes a switch. Named branch unchanged. Positional now requires value or valueHint. Unknown or empty type is rejected. Two new errors in constants.go, references invalid-argument-type and positional-argument-value-or-hint-required.
Why not full schema validation at publish: the enhanced validation design doc gates that behind the anyOf error consolidation (Phase 3), raw anyOf failures on arguments come out as a noisy multi-error cluster. These two checks give one clean message each and are one case arm each, easy to delete at Phase 5 when schema-first lands. Until then publish and edit just agree with what /v0/validate already reports.
Heads up on two consequences: editing a stored server that still carries one of these legacy arguments will 422 until the argument is fixed, and the importer skips invalid servers with a warning, so a re-seed from a live dump drops those entries instead of failing. Also #1339 adds a check inside the named branch of the same function, whichever lands second has a small conflict to resolve there, happy to do it from my side.
Test
Tests first, they fail on main, the handler regression actually publishes an empty-type argument with 200 on main and gets 422 with the fix. On the branch: go test ./internal/... all green including the handler tests against postgres, golangci-lint v2.13.1 (what CI pins) 0 issues, make validate passes. Two existing fixtures had positional args with neither value nor valueHint, they got a valueHint since no published schema version ever allowed that shape. New error paths and references are pinned in validation_detailed_test.go.
AI Disclosure
AI assistance (Claude) was used for issue research and during development. All changes were reviewed and tested by the author.