diff options
| author | Hana Kim <hyangah@gmail.com> | 2026-04-12 22:10:16 -0400 |
|---|---|---|
| committer | Hyang-Ah Hana Kim <hyangah@gmail.com> | 2026-04-13 11:42:30 -0700 |
| commit | aca48c6d9dcc4892027cdad3243527882201358b (patch) | |
| tree | 8827e389417a4d7ef4344fd7f28dac0c6be19934 | |
| parent | 70e5087371296e2632232f4a3a795f124c73baf3 (diff) | |
| download | go-x-pkgsite-aca48c6d9dcc4892027cdad3243527882201358b.tar.xz | |
internal/api: improve ambiguous package path resolution
Instead of falling back to UnknownModulePath, we now:
1. Query all candidate module paths.
2. Filter out candidates where the database fell back to a different module path (preventing false positives like google.golang.org).
3. Filter out deprecated or retracted candidates if at least one good candidate exists.
4. Return 400 if ambiguity remains among good candidates.
This commit fixes the issue where /v1/package/google.golang.org/adk/agent was returning HTTP 400 because all candidate module paths matched:
```
{
"code":400,
"message":"ambiguous package path",
"candidates":[
{"modulePath":"google.golang.org/adk/agent","packagePath":"google.golang.org/adk/agent"},
{"modulePath":"google.golang.org/adk","packagePath":"google.golang.org/adk/agent"},
{"modulePath":"google.golang.org","packagePath":"google.golang.org/adk/agent"}]
}
```
Change-Id: I3ea24bca5144d536490019efd85fb597da214029
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/766380
kokoro-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
| -rw-r--r-- | doc/pkginfo.md | 69 | ||||
| -rw-r--r-- | internal/api/api.go | 95 | ||||
| -rw-r--r-- | internal/api/api_test.go | 90 |
3 files changed, 223 insertions, 31 deletions
diff --git a/doc/pkginfo.md b/doc/pkginfo.md new file mode 100644 index 00000000..9cec9084 --- /dev/null +++ b/doc/pkginfo.md @@ -0,0 +1,69 @@ +# pkginfo + +A command-line interface for querying pkg.go.dev. +(Related: https://go.dev/issue/76718). + +## Motivation + +The pkg.go.dev website exposes a v1 JSON API for package metadata, but currently no command-line tool provides access to it. Developers must use a browser or interact directly with the REST API. Similarly, AI coding agents must either scrape web pages or implement custom API clients. `pkginfo` fills this gap by providing a lightweight CLI that queries the API and prints results for both humans and automated tools. + +## Relationship to existing tools + +- **`go doc`** renders documentation for packages available locally. `pkginfo` does not replace it for reading local documentation. +- **`cmd/pkgsite`** is a local documentation server. Its data source does not support global information like reverse dependencies or full ecosystem search. +- **`pkginfo`** provides access to information `go doc` cannot reach: version listings, vulnerability reports, reverse dependencies, licenses, documentation +of modules/packages, and search results for packages not yet downloaded. + +Rule of thumb: Use `go doc` for local code; use `pkginfo` for ecosystem discovery and metadata. + +## Commands + +### Package info +`pkginfo [flags] <package>[@version]` + +Example: +``` +$ pkginfo encoding/json +encoding/json (standard library) + Module: std + Version: go1.24.2 (latest) +``` + +Flags: +- `--doc`: Render doc. +- `--examples`: Include examples (requires `--doc`). +- `--imports`: List imports. +- `--imported-by`: List reverse dependencies. +- `--symbols`: List exported symbols. +- `--licenses`: Show licenses. +- `--module=<path>`: Disambiguate module. + +### Module info +`pkginfo module [flags] <module>[@version]` + +Flags: +- `--readme`: Print README. +- `--licenses`: List licenses. +- `--versions`: List versions. +- `--vulns`: List vulnerabilities. +- `--packages`: List packages in module. + +### Search +`pkginfo search [flags] <query>` + +Flags: +- `--symbol=<name>`: Search for symbol. + +## Common Flags +- `--json`: Output structured JSON. +- `--limit=N`: Max results. +- `--server=URL`: API server URL. + +## Details +- Ambiguous path: CLI shows candidates. Use `--module` to resolve. +- Pagination: JSON returns `nextPageToken`. Use `--token` to continue. + +## Status and Implementation +- **Experimental**: This tool is currently a prototype. +- **Minimal Dependencies**: To facilitate potential migration to other repositories (e.g. `x/tools`), the tool depends only on the Go standard library. If it stays in this repository, we need to plan for the release/tagging policy. +- **Duplicated Types**: API response types are duplicated in the tool's source instead of imported from `pkgsite` for now. We ruled out releasing a full SDK because the REST API is simple enough to consume directly. This keeps the tool self-contained. diff --git a/internal/api/api.go b/internal/api/api.go index 87339e19..0f610faf 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "net/http" + "slices" "strconv" "strings" "time" @@ -451,48 +452,82 @@ func trimPath(r *http.Request, prefix string) string { // resolveModulePath determines the correct module path for a given package path and version. // If the module path is not provided, it searches through potential candidate module paths -// derived from the package path. If multiple valid modules contain the package, it returns -// a list of candidates to help the user disambiguate the request. +// derived from the package path. +// +// Resolution logic: +// 1. Use internal.CandidateModulePaths(pkgPath) to get potential candidates (ordered longest first). +// 2. Fetch UnitMeta for each candidate that exists in the data source. +// 3. Check if um.ModulePath == mp (where mp is the candidate module path). If not, ignore it +// (this handles the case where GetUnitMeta falls back to another module when the requested +// module does not exist). +// 4. Filter candidates by eliminating those that are deprecated or retracted. +// 5. If exactly one candidate remains after filtering, return it (HTTP 200). +// 6. If multiple candidates remain, return HTTP 400 with the list of candidates (ambiguity). +// 7. If all candidates are eliminated (e.g., all are deprecated or retracted), fall back to +// the longest matching candidate among those that exist (HTTP 200). func resolveModulePath(r *http.Request, ds internal.DataSource, pkgPath, modulePath, requestedVersion string) (*internal.UnitMeta, error) { if requestedVersion == "" { requestedVersion = version.Latest } - if modulePath == "" { - // Handle potential ambiguity if module is not specified. - candidates := internal.CandidateModulePaths(pkgPath) - var validCandidates []Candidate - var foundUM *internal.UnitMeta - for _, mp := range candidates { - // Check if this module actually exists and contains the package at the requested version. - if m, err := ds.GetUnitMeta(r.Context(), pkgPath, mp, requestedVersion); err == nil { - foundUM = m - validCandidates = append(validCandidates, Candidate{ - ModulePath: mp, - PackagePath: pkgPath, - }) - } else if !errors.Is(err, derrors.NotFound) { - return nil, err - } + if modulePath != "" { + um, err := ds.GetUnitMeta(r.Context(), pkgPath, modulePath, requestedVersion) + if err != nil { + return nil, err } + return um, nil + } - if len(validCandidates) > 1 { - return nil, &Error{ - Code: http.StatusBadRequest, - Message: "ambiguous package path", - Candidates: validCandidates, + candidates := internal.CandidateModulePaths(pkgPath) + var validCandidates []*internal.UnitMeta + for _, mp := range candidates { + if um, err := ds.GetUnitMeta(r.Context(), pkgPath, mp, requestedVersion); err == nil { + // Critical check: ensure the DB actually found the candidate module we requested. + // GetUnitMeta falls back to the best match if the requested module doesn't exist, + // which could lead to false positives (e.g. google.golang.org matching because it + // falls back to google.golang.org/adk/agent). + if um.ModulePath == mp { + validCandidates = append(validCandidates, um) } + } else if !errors.Is(err, derrors.NotFound) { + return nil, err } - if len(validCandidates) == 0 { - return nil, derrors.NotFound + } + + if len(validCandidates) == 0 { + return nil, derrors.NotFound + } + + // Filter candidates based on signals (deprecation, retraction). + goodCandidates := slices.Clone(validCandidates) + goodCandidates = slices.DeleteFunc(goodCandidates, func(um *internal.UnitMeta) bool { + return um.Deprecated || um.Retracted + }) + + switch len(goodCandidates) { + case 1: + return goodCandidates[0], nil + case 0: + // If all candidates are deprecated or retracted, fall back to the longest match. + // Since candidates are ordered longest first, validCandidates[0] is the longest match. + return validCandidates[0], nil + default: + return nil, &Error{ + Code: http.StatusBadRequest, + Message: "ambiguous package path", + Candidates: makeCandidates(goodCandidates), } - return foundUM, nil } +} - um, err := ds.GetUnitMeta(r.Context(), pkgPath, modulePath, requestedVersion) - if err != nil { - return nil, err +func makeCandidates(ums []*internal.UnitMeta) []Candidate { + var r []Candidate + for _, um := range ums { + r = append(r, Candidate{ + ModulePath: um.ModulePath, + PackagePath: um.Path, + }) } - return um, nil + return r } // Values for the Cache-Control header. diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 285005de..64b891c7 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -23,6 +23,22 @@ import ( "golang.org/x/pkgsite/internal/vuln" ) +type fallbackDataSource struct { + internal.DataSource + fallbackMap map[string]string // requested module -> resolved module +} + +func (ds fallbackDataSource) GetUnitMeta(ctx context.Context, path, requestedModulePath, requestedVersion string) (*internal.UnitMeta, error) { + if resolved, ok := ds.fallbackMap[requestedModulePath]; ok { + um, err := ds.DataSource.GetUnitMeta(ctx, path, resolved, requestedVersion) + if err != nil { + return nil, err + } + return um, nil + } + return ds.DataSource.GetUnitMeta(ctx, path, requestedModulePath, requestedVersion) +} + func TestServePackage(t *testing.T) { ctx := context.Background() ds := fakedatasource.New() @@ -141,6 +157,7 @@ func TestServePackage(t *testing.T) { url string wantStatus int want any // Can be *Package or *Error + overrideDS internal.DataSource }{ { name: "basic metadata", @@ -329,12 +346,83 @@ func TestServePackage(t *testing.T) { Imports: []string{pkgPath}, }, }, + { + name: "fallback prevention (false positive candidate)", + url: "/v1/package/example.com/a/b?version=v1.2.3", + wantStatus: http.StatusBadRequest, + want: &Error{ + Code: http.StatusBadRequest, + Message: "ambiguous package path", + Candidates: []Candidate{ + {ModulePath: "example.com/a/b", PackagePath: "example.com/a/b"}, + {ModulePath: "example.com/a", PackagePath: "example.com/a/b"}, + }, + }, + overrideDS: &fallbackDataSource{ + DataSource: ds, + fallbackMap: map[string]string{ + "example.com": "example.com/a/b", // simulate fallback + }, + }, + }, + { + name: "deprecation filtering", + url: "/v1/package/example.com/a/b?version=v1.2.3", + wantStatus: http.StatusOK, + want: &Package{ + Path: pkgPath, + ModulePath: modulePath2, // picked because modulePath1 is deprecated + ModuleVersion: version, + Synopsis: "Synopsis for " + modulePath2, + IsLatest: true, + GOOS: "linux", + GOARCH: "amd64", + }, + overrideDS: func() internal.DataSource { + newDS := fakedatasource.New() + for _, mp := range []string{modulePath1, modulePath2} { + u := &internal.Unit{ + UnitMeta: internal.UnitMeta{ + Path: pkgPath, + ModuleInfo: internal.ModuleInfo{ + ModulePath: mp, + Version: version, + LatestVersion: version, + Deprecated: mp == modulePath1, + }, + Name: "b", + }, + Documentation: []*internal.Documentation{ + { + GOOS: "linux", + GOARCH: "amd64", + Synopsis: "Synopsis for " + mp, + }, + }, + } + newDS.MustInsertModule(ctx, &internal.Module{ + ModuleInfo: internal.ModuleInfo{ + ModulePath: mp, + Version: version, + LatestVersion: version, + Deprecated: mp == modulePath1, + }, + Units: []*internal.Unit{u}, + }) + } + return newDS + }(), + }, } { t.Run(test.name, func(t *testing.T) { r := httptest.NewRequest("GET", test.url, nil) w := httptest.NewRecorder() - if err := ServePackage(w, r, ds); err != nil { + var currentDS internal.DataSource = ds + if test.overrideDS != nil { + currentDS = test.overrideDS + } + if err := ServePackage(w, r, currentDS); err != nil { ServeError(w, r, err) } |
