Fix resolve for local agent build, it wasn't using auth so failed when an agent had MCP, Prompt, or Skill#440
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes authenticated registry resolution during arctl agent run so registry-backed MCP servers (and prompts) can be fetched from registries that require auth.
Changes:
- Inject the authenticated
internal/client.Clientinto agent manifest resolution viaagentutils.SetRegistryAPIClient. - Update MCP server resolution to use
Client.GetServerVersioninstead of an unauthenticated registry client. - Align registry server translation to accept the API’s
apiv0.ServerJSONtype directly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cli/root.go | Stores the authenticated API client in agent utils during CLI pre-run. |
| internal/cli/agent/utils/registry_translator.go | Switches translation input type to apiv0.ServerJSON and simplifies mapping. |
| internal/cli/agent/utils/registry_resolver.go | Uses the authenticated API client for resolving registry MCP servers and prompts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| apiClient := registryAPIClient | ||
| if apiClient == nil { | ||
| apiClient = client.NewClient(registryURL, "") | ||
| } | ||
|
|
||
| serverResp, err := apiClient.GetServerVersion(serverName, serverVersion) |
| @@ -230,10 +245,9 @@ func ResolveManifestPrompts(manifest *models.AgentManifest, verbose bool) ([]com | |||
| i, ref.Name, promptName, promptVersion, registryURL) | |||
| } | |||
|
|
|||
| apiClient, ok := clients[registryURL] | |||
| if !ok { | |||
| apiClient := registryAPIClient | |||
| if apiClient == nil { | |||
| apiClient = client.NewClient(registryURL, "") | |||
| clients[registryURL] = apiClient | |||
| } | |||
| apiClient := registryAPIClient | ||
| if apiClient == nil { | ||
| apiClient = client.NewClient(registryURL, "") | ||
| clients[registryURL] = apiClient | ||
| } |
|
This code is actually slated to be likely cleaned up with later refactor to remove imperative CLI. Merging this will enable some things i am doing but we don't need to be very critical. |
|
This pull request has been marked as stale due to no activity in the last 14 days. It will be closed in 3 days unless it is tagged "no stalebot" or other activity occurs. |
|
This pull request has been closed due to inactivity. |
Description
Resolve #439
Change Type
/kind fix
Changelog
Additional Notes