Skip to content

feat: add GET endpoint by primitive kind#2907

Open
Yuan325 wants to merge 1 commit into
admin-getfrom
admin-get-primitive
Open

feat: add GET endpoint by primitive kind#2907
Yuan325 wants to merge 1 commit into
admin-getfrom
admin-get-primitive

Conversation

@Yuan325
Copy link
Copy Markdown
Contributor

@Yuan325 Yuan325 commented Mar 31, 2026

This will be the path to get a list of primitives that are configured within Toolbox.
For example, /admin/source will return a list of sources. The available api includes the following (non-case sensitive):

  • /admin/source
  • /admin/authService or /admin/authservice
  • /admin/tool
  • /admin/toolset

An invalid primitive name will result in a bad request error.

@Yuan325 Yuan325 requested a review from a team as a code owner March 31, 2026 03:54
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new administrative endpoint to retrieve lists of resource names by category (sources, auth services, tools, etc.) and adds corresponding methods to the ResourceManager. Feedback suggests improving the getPrimitive handler by adding instrumentation for consistency, renaming variables for clarity, and enhancing error messages. Additionally, it is recommended to use slices.Sorted when collecting map keys in the ResourceManager to ensure deterministic API responses and prevent flaky tests.

Comment thread internal/server/admin.go
Comment thread internal/server/resources/resources.go Outdated
Comment thread internal/server/resources/resources.go Outdated
Comment thread internal/server/resources/resources.go Outdated
Comment thread internal/server/resources/resources.go Outdated
Comment thread internal/server/resources/resources.go Outdated
Comment thread internal/server/resources/resources.go Outdated
@Yuan325 Yuan325 force-pushed the admin-get-primitive branch from 6e28433 to 6b31505 Compare March 31, 2026 17:50
func (r *ResourceManager) GetSources() []string {
r.mu.RLock()
defer r.mu.RUnlock()
return slices.Sorted(maps.Keys(r.sources))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this may return nil if sorting empty list. we should probably ensure we are returning an empty list in that case.

Comment thread internal/server/admin.go
r.Put("/{kind}/{name}", func(w http.ResponseWriter, r *http.Request) { createOrUpdatePrimitives(s, w, r) })
r.Delete("/{kind}/{name}", func(w http.ResponseWriter, r *http.Request) { deletePrimitives(s, w, r) })
r.Get("/{kind}/{name}", func(w http.ResponseWriter, r *http.Request) { getPrimitiveByName(s, w, r) })
r.Route("/{kind}", func(r chi.Router) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to stack these like
1 r.Route("/{kind}", func(r chi.Router) {
2 r.Get("/", ...)
3 r.Get("/{name}", ...)
4 r.Put("/{name}", ...)
5 r.Delete("/{name}", ...)
6 })

return slices.Sorted(maps.Keys(r.toolsets))
}

func (r *ResourceManager) GetPrompts() []string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me why we do prompts and not promptset as well?

@duwenxin99 duwenxin99 assigned averikitsch and unassigned duwenxin99 May 5, 2026
@averikitsch averikitsch added the priority: p2 Moderately-important priority. Fix may not be included in next release. label May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p2 Moderately-important priority. Fix may not be included in next release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants