Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pkg/hookdeck/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,13 @@ func (c *Client) withoutProjectScope() *Client {

// ValidateAPIKey validates an API key and returns user/project information
func (c *Client) ValidateAPIKey() (*ValidateAPIKeyResponse, error) {
res, err := c.clientForCLIAuthValidate().Get(context.Background(), APIPathPrefix+"/cli-auth/validate", "", nil)
return c.ValidateAPIKeyContext(context.Background())
}

// ValidateAPIKeyContext is ValidateAPIKey bounded by ctx, for callers that must
// not wait indefinitely on the API.
func (c *Client) ValidateAPIKeyContext(ctx context.Context) (*ValidateAPIKeyResponse, error) {
res, err := c.clientForCLIAuthValidate().Get(ctx, APIPathPrefix+"/cli-auth/validate", "", nil)
if err != nil {
return nil, err
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/hookdeck/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ type Project struct {
}

func (c *Client) ListProjects() ([]Project, error) {
res, err := c.clientForCLIAuthValidate().Get(context.Background(), APIPathPrefix+"/projects", "", nil)
return c.ListProjectsContext(context.Background())
}

// ListProjectsContext is ListProjects bounded by ctx, for callers that must not
// wait indefinitely on the API.
func (c *Client) ListProjectsContext(ctx context.Context) ([]Project, error) {
res, err := c.clientForCLIAuthValidate().Get(ctx, APIPathPrefix+"/projects", "", nil)
if err != nil {
return []Project{}, err
}
Expand Down
32 changes: 29 additions & 3 deletions pkg/mcpcore/project_display.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,31 @@
package mcpcore

import (
"context"
"time"

"github.com/hookdeck/hookdeck-cli/pkg/hookdeck"
"github.com/hookdeck/hookdeck-cli/pkg/project"
)

// projectDisplayLookupTimeout bounds the whole display-name lookup. The lookup
// runs before every tool call until it succeeds, including calls that never
// reach the API (help, a write refused in read-only mode, a rejected argument),
// and the API client has no deadline of its own. Without this bound one slow
// API response held the tool call open for as long as the API took: the client
// saw initialize answered and then nothing. The name is cosmetic, so giving up
// costs only an id where a name would have been, and the next call tries again.
// A variable so tests can shorten it.
var projectDisplayLookupTimeout = 5 * time.Second

// FillProjectDisplayNameIfNeeded sets target.ProjectOrg and target.ProjectName
// when target has an API key and project id but no cached org/name (typical
// after loading the profile from disk). Fails silently on API errors. Stdio MCP
// invokes tools sequentially, so this is safe without locking.
//
// The lookup gives up after projectDisplayLookupTimeout, or sooner when ctx is
// done, so it can never hold up the tool call it runs ahead of.
//
// lookup is the client account-level requests are made from, which is not always
// target: a product API served from its own host does not answer them, so the
// lookup has to go to the account API.
Expand All @@ -19,26 +35,36 @@ import (
// projects only works for a user-associated key, so a project-scoped key from
// `hookdeck ci` — a common way to configure an MCP server — would otherwise
// leave the name empty and callers with nothing but an opaque id to show.
func FillProjectDisplayNameIfNeeded(lookup, target *hookdeck.Client) {
func FillProjectDisplayNameIfNeeded(ctx context.Context, lookup, target *hookdeck.Client) {
if lookup == nil || target == nil || target.APIKey == "" || target.ProjectID == "" {
return
}
if target.ProjectName != "" || target.ProjectOrg != "" {
return
}
if ctx == nil {
ctx = context.Background()
}
ctx, cancel := context.WithTimeout(ctx, projectDisplayLookupTimeout)
defer cancel()

// The key's own project, available whatever kind of key it is.
if response, err := lookup.ValidateAPIKey(); err == nil && response.ProjectID == target.ProjectID {
if response, err := lookup.ValidateAPIKeyContext(ctx); err == nil && response.ProjectID == target.ProjectID {
target.ProjectName = response.ProjectName
target.ProjectOrg = response.OrganizationName
if target.ProjectName != "" || target.ProjectOrg != "" {
return
}
}

// Out of time: the listing would fail too.
if ctx.Err() != nil {
return
}

// The active project differs from the key's, so it has to be looked up.
// Only a user-associated key can do this.
projects, err := lookup.ListProjects()
projects, err := lookup.ListProjectsContext(ctx)
if err != nil {
return
}
Expand Down
103 changes: 98 additions & 5 deletions pkg/mcpcore/project_display_test.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,111 @@
package mcpcore

import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync/atomic"
"testing"
"time"

mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp"

"github.com/hookdeck/hookdeck-cli/pkg/hookdeck"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// stalledAPI answers nothing until the request is abandoned, which is what a
// slow API looks like to a client with no deadline of its own.
func stalledAPI(t *testing.T) (*hookdeck.Client, *atomic.Int64) {
t.Helper()
var requests atomic.Int64
release := make(chan struct{})
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requests.Add(1)
select {
case <-r.Context().Done():
case <-release:
}
}))
t.Cleanup(func() {
close(release)
server.Close()
})
baseURL, err := url.Parse(server.URL)
require.NoError(t, err)
return &hookdeck.Client{BaseURL: baseURL, APIKey: "k", ProjectID: "tm_1"}, &requests
}

func shortenProjectDisplayLookup(t *testing.T, d time.Duration) {
t.Helper()
previous := projectDisplayLookupTimeout
projectDisplayLookupTimeout = d
t.Cleanup(func() { projectDisplayLookupTimeout = previous })
}

// TestFillProjectDisplayName_GivesUpOnAStalledAPI: the lookup is cosmetic, so a
// slow API must cost at most the timeout, not however long the API takes.
func TestFillProjectDisplayName_GivesUpOnAStalledAPI(t *testing.T) {
shortenProjectDisplayLookup(t, 200*time.Millisecond)
client, requests := stalledAPI(t)

start := time.Now()
FillProjectDisplayNameIfNeeded(context.Background(), client, client)

assert.Less(t, time.Since(start), 3*time.Second, "the lookup must give up at its timeout")
assert.GreaterOrEqual(t, requests.Load(), int64(1), "the lookup should have been attempted")
assert.Empty(t, client.ProjectName, "a lookup that gave up leaves the name for the next call to fill")
assert.Empty(t, client.ProjectOrg)
}

// TestToolCallIsNotHeldByAStalledDisplayNameLookup is the case the acceptance
// suite kept hitting: a tool that never needs the API (here, one standing in for
// a write refused in read-only mode) answered nothing, because the lookup ahead
// of it waited on the API with no deadline.
func TestToolCallIsNotHeldByAStalledDisplayNameLookup(t *testing.T) {
shortenProjectDisplayLookup(t, 200*time.Millisecond)
client, _ := stalledAPI(t)
s := &Server{client: client, accountClient: client}

refusal := mcpsdk.ToolHandler(func(ctx context.Context, req *mcpsdk.CallToolRequest) (*mcpsdk.CallToolResult, error) {
return &mcpsdk.CallToolResult{IsError: true}, nil
})

done := make(chan *mcpsdk.CallToolResult, 1)
go func() {
result, _ := s.wrapWithTelemetry("gateway_sources_read", refusal)(context.Background(), newCallToolRequest(`{"action":"delete"}`))
done <- result
}()

select {
case result := <-done:
require.NotNil(t, result)
assert.True(t, result.IsError)
case <-time.After(5 * time.Second):
t.Fatal("the tool call is still waiting on the display-name lookup")
}
}

// TestFillProjectDisplayName_StopsWhenTheCallIsCancelled: a client that cancels
// the tool call must not leave the lookup running to its own timeout.
func TestFillProjectDisplayName_StopsWhenTheCallIsCancelled(t *testing.T) {
shortenProjectDisplayLookup(t, time.Minute)
client, _ := stalledAPI(t)

ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
defer cancel()

start := time.Now()
FillProjectDisplayNameIfNeeded(ctx, client, client)

assert.Less(t, time.Since(start), 3*time.Second, "cancelling the call must end the lookup")
assert.Empty(t, client.ProjectName)
}

func TestFillProjectDisplayNameIfNeeded_SetsNameFromAPI(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/2026-09-01/projects" {
Expand All @@ -32,14 +125,14 @@ func TestFillProjectDisplayNameIfNeeded_SetsNameFromAPI(t *testing.T) {
APIKey: "k",
ProjectID: "proj_x",
}
FillProjectDisplayNameIfNeeded(client, client)
FillProjectDisplayNameIfNeeded(context.Background(), client, client)
require.Equal(t, "Acme", client.ProjectOrg)
require.Equal(t, "production", client.ProjectName)
}

func TestFillProjectDisplayNameIfNeeded_NoOpWhenNameSet(t *testing.T) {
client := &hookdeck.Client{ProjectID: "p", ProjectName: "already"}
FillProjectDisplayNameIfNeeded(client, client)
FillProjectDisplayNameIfNeeded(context.Background(), client, client)
require.Equal(t, "already", client.ProjectName)
}

Expand Down Expand Up @@ -72,7 +165,7 @@ func TestFillProjectDisplayNameIfNeeded_LooksUpThroughTheAccountClient(t *testin
account := &hookdeck.Client{BaseURL: accountURL, APIKey: "k", ProjectID: "proj_x"}
product := &hookdeck.Client{BaseURL: productURL, APIKey: "k", ProjectID: "proj_x"}

FillProjectDisplayNameIfNeeded(account, product)
FillProjectDisplayNameIfNeeded(context.Background(), account, product)
require.Equal(t, "Acme", product.ProjectOrg)
require.Equal(t, "production", product.ProjectName)
}
Expand Down Expand Up @@ -100,7 +193,7 @@ func TestFillProjectDisplayName_ProjectScopedKey(t *testing.T) {
require.NoError(t, err)
client := &hookdeck.Client{BaseURL: baseURL, APIKey: "ci-key", ProjectID: "tm_1"}

FillProjectDisplayNameIfNeeded(client, client)
FillProjectDisplayNameIfNeeded(context.Background(), client, client)

assert.Equal(t, "cli outpost testing", client.ProjectName)
assert.Equal(t, "Automated Testing", client.ProjectOrg)
Expand All @@ -126,7 +219,7 @@ func TestFillProjectDisplayName_FallsBackToListing(t *testing.T) {
require.NoError(t, err)
client := &hookdeck.Client{BaseURL: baseURL, APIKey: "user-key", ProjectID: "tm_1"}

FillProjectDisplayNameIfNeeded(client, client)
FillProjectDisplayNameIfNeeded(context.Background(), client, client)

assert.Equal(t, "the active one", client.ProjectName, "the key's own project must not be used when it is not the active one")
}
2 changes: 1 addition & 1 deletion pkg/mcpcore/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func (s *Server) wrapWithTelemetry(toolName string, handler mcpsdk.ToolHandler)
}
}()

FillProjectDisplayNameIfNeeded(s.accountClient, s.client)
FillProjectDisplayNameIfNeeded(ctx, s.accountClient, s.client)

return handler(ctx, req)
}
Expand Down
Loading
Loading