From 14bfccac1ce5f912b7ddbe416da925d4c1d488ee Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 21 Oct 2026 18:05:03 -0400 Subject: [PATCH] Support branches in the existing MCP tools db_query and db_schema now resolve service_id through the API's resolve endpoint, so behind the experimental gate they accept a branch ID as well as a service or read replica set ID, and a paused or unready branch points at the branch tools. service_get returns a fluid service's branch count behind the gate. service_delete names the branches deleted along with a fluid service in its PROD prompt and its result, regardless of the gate. --- CLAUDE.md | 2 +- internal/cmd/integration_test.go | 2 +- internal/common/connection_target.go | 50 ++----- internal/common/connection_target_test.go | 166 ++++++++++++++-------- internal/mcp/db_extensions_list.go | 14 +- internal/mcp/db_extensions_list_test.go | 61 +++++--- internal/mcp/db_query.go | 15 +- internal/mcp/db_query_test.go | 65 ++++++--- internal/mcp/db_schema.go | 15 +- internal/mcp/db_schema_test.go | 31 ++-- internal/mcp/errors.go | 16 ++- internal/mcp/main_test.go | 31 ++-- internal/mcp/server.go | 12 +- internal/mcp/server_test.go | 2 +- internal/mcp/service_delete.go | 86 ++++++++--- internal/mcp/service_delete_test.go | 125 ++++++++++++++++ internal/mcp/service_get.go | 10 ++ internal/mcp/service_get_test.go | 85 +++++++++-- internal/mcp/utils.go | 22 +++ 19 files changed, 587 insertions(+), 223 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7c09c08e..b2f5fd98 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,7 +143,7 @@ A command that identifies a service takes a **ref** — its ID, a read replica s - Help text shows `[name-or-id]` and says the service "can be given by ID or name". Shell completion offers names, falling back to IDs when what is typed matches an ID instead. - A name is accepted only as an argument. A configured default must be an ID, and one that resolves by name is refused. The `--service-id` flag completion only offers IDs. - Destructive commands accept a name, but their confirmation prompt takes only the ID. -- MCP tools stay IDs-only — an intentional divergence, documented at `setServiceIDSchemaProperties`. A tool that goes through the resolve endpoint (the branch tools) refuses a match by name. +- MCP tools stay IDs-only — an intentional divergence, documented at `setServiceIDSchemaProperties`. A tool that goes through the resolve endpoint (the branch tools, `db_query`, `db_schema`, `db_extensions_list`) refuses a match by name. ## Output diff --git a/internal/cmd/integration_test.go b/internal/cmd/integration_test.go index d38a28e6..d1069ce8 100644 --- a/internal/cmd/integration_test.go +++ b/internal/cmd/integration_test.go @@ -493,7 +493,7 @@ func TestServiceLifecycleIntegration(t *testing.T) { if err != nil { t.Fatalf("failed to build API client: %v", err) } - target, err := common.ResolveConnectionTargetByID(t.Context(), client, projectID, serviceID) + target, err := common.ResolveConnectionTargetByID(t.Context(), client, projectID, serviceID, false) if err != nil { t.Fatalf("failed to resolve connection target: %v", err) } diff --git a/internal/common/connection_target.go b/internal/common/connection_target.go index 52ed313c..26764c76 100644 --- a/internal/common/connection_target.go +++ b/internal/common/connection_target.go @@ -188,6 +188,19 @@ func ResolveConnectionTarget(ctx context.Context, client api.ClientWithResponses } } +// ResolveConnectionTargetByID is ResolveConnectionTarget for a caller that +// takes only IDs: a ref that matched by name is reported as not found. +func ResolveConnectionTargetByID(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string, allowBranches bool) (*ConnectionTarget, error) { + target, err := ResolveConnectionTarget(ctx, client, projectID, id, allowBranches) + if err != nil { + return nil, err + } + if target.ID != id { + return nil, ExitWithCode(ExitServiceNotFound, fmt.Errorf("%s '%s' not found", connectionTargetKind(allowBranches), id)) + } + return target, nil +} + // connectionTargetKind names what ResolveConnectionTarget looks for, in its // errors. Branches are left out unless they are allowed. func connectionTargetKind(allowBranches bool) string { @@ -197,43 +210,6 @@ func connectionTargetKind(allowBranches bool) string { return "service or read replica set" } -// ResolveConnectionTargetByID fetches the service named by id — which may be a -// primary service ID or a read replica set ID, both of which GetService -// resolves — and works out whose credentials it uses. Callers that already -// hold the service use NewConnectionTarget instead. -func ResolveConnectionTargetByID(ctx context.Context, client api.ClientWithResponsesInterface, projectID, id string) (*ConnectionTarget, error) { - service, err := GetService(ctx, client, projectID, id) - if err != nil { - return nil, err - } - return NewConnectionTarget(ctx, client, projectID, *service) -} - -// NewConnectionTarget returns the ConnectionTarget of an already-fetched -// service. A standby read replica connects to its own endpoint but shares the -// parent primary's credentials, so the parent is fetched here. -func NewConnectionTarget(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, service api.Service) (*ConnectionTarget, error) { - target := NewServiceConnectionTarget(service) - if !IsReadReplica(service) { - return target, nil - } - target.Kind = TargetReadReplica - - // A replica with no parent recorded has nowhere else to look, so it stands - // in as its own credential owner. - parentID := util.DerefStr(service.ForkedFrom.ServiceID) - if parentID == "" { - return target, nil - } - - parent, err := GetService(ctx, client, projectID, parentID) - if err != nil { - return nil, fmt.Errorf("failed to fetch parent service %q for read replica: %w", parentID, err) - } - target.Credentials = ServiceCredentials(*parent) - return target, nil -} - // environmentTag reads an optional environment tag. Anything but PROD reads // as DEV, since the tag is optional and treating unknowns as protected would // have read_only=prod block untagged services. diff --git a/internal/common/connection_target_test.go b/internal/common/connection_target_test.go index a3f57205..2293e849 100644 --- a/internal/common/connection_target_test.go +++ b/internal/common/connection_target_test.go @@ -15,20 +15,48 @@ import ( "github.com/timescale/tiger-cli/internal/util" ) -// serviceTestClient builds an API client backed by an httptest server that -// serves the getService endpoint from services keyed by service ID (404 when -// absent). -func serviceTestClient(t *testing.T, services map[string]api.Service) *api.ClientWithResponses { +// resolveTestClient builds an API client backed by an httptest server that +// answers the resolve endpoint from resolved, keyed by ref, and the getService +// endpoint from services, keyed by service ID (404 when absent). Each resolved +// value is a service, branch, or read replica set, encoded with the given +// discriminator. +func resolveTestClient(t *testing.T, resolved map[string]resolvedFixture, services map[string]api.Service) *api.ClientWithResponses { t.Helper() + notFound := func(w http.ResponseWriter) { + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(map[string]string{"message": "not found"}) + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") parts := strings.Split(strings.Trim(r.URL.Path, "/"), "/") - id := parts[len(parts)-1] - svc, ok := services[id] + last := parts[len(parts)-1] + + if last == "resolve" { + fixture, ok := resolved[r.URL.Query().Get("ref")] + if !ok { + notFound(w) + return + } + raw, err := json.Marshal(fixture.value) + if err != nil { + t.Errorf("failed to encode fixture: %v", err) + return + } + var body map[string]any + if err := json.Unmarshal(raw, &body); err != nil { + t.Errorf("failed to decode fixture: %v", err) + return + } + body["type"] = fixture.typ + _ = json.NewEncoder(w).Encode(body) + return + } + + svc, ok := services[last] if !ok { - w.WriteHeader(http.StatusNotFound) - _ = json.NewEncoder(w).Encode(map[string]string{"message": "service not found"}) + notFound(w) return } _ = json.NewEncoder(w).Encode(svc) @@ -42,6 +70,12 @@ func serviceTestClient(t *testing.T, services map[string]api.Service) *api.Clien return client } +// resolvedFixture is one answer of the resolve endpoint in resolveTestClient. +type resolvedFixture struct { + typ api.ResolveType + value any +} + func primaryService() api.Service { host := "svcprimary.example.com" port := 5432 @@ -66,31 +100,37 @@ func replicaSet() api.ReadReplicaSet { } } -// replicaService returns a standby read replica of primaryService, shaped -// the way GetService returns it when given a read replica set ID. -func replicaService() api.Service { - host := "replica.example.com" - port := 5432 - return api.Service{ - ServiceID: "rep1234567", - ProjectID: "proj1", - Name: "reporting-replica", - Endpoint: &api.Endpoint{Host: &host, Port: &port}, - ForkedFrom: &api.ForkSpec{ - IsStandby: new(true), - ProjectID: new("proj1"), - ServiceID: new("svcprimary"), - }, - } -} - +// TestResolveConnectionTargetByID is helper-level because the MCP db tools +// that call it can't reach the branch and replica cases without a live +// connection past it. func TestResolveConnectionTargetByID(t *testing.T) { primary := primaryService() - replica := replicaService() - client := serviceTestClient(t, map[string]api.Service{"svcprimary": primary, "rep1234567": replica}) + replicaHost := "replica.example.com" + replica := api.ReadReplicaSet{ + ID: "rep1234567", + ServiceID: "svcprimary", + Name: "reporting-replica", + Status: api.ReadReplicaSetStatusActive, + Endpoint: &api.Endpoint{Host: &replicaHost, Port: new(5432)}, + } + branch := api.Branch{ + ID: "br12345678", + ProjectID: "proj1", + ServiceID: "svcprimary", + Name: "my-branch", + Status: api.DeployStatusREADY, + Endpoint: &api.Endpoint{Host: new("br12345678.example.com"), Port: new(5432)}, + } + client := resolveTestClient(t, map[string]resolvedFixture{ + "svcprimary": {api.ResolveTypeSERVICE, primary}, + "my-db": {api.ResolveTypeSERVICE, primary}, + "rep1234567": {api.ResolveTypeREADREPLICASET, replica}, + "br12345678": {api.ResolveTypeBRANCH, branch}, + }, map[string]api.Service{"svcprimary": primary}) + ctx := context.Background() // Primary ID → primary target. - target, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "svcprimary") + target, err := ResolveConnectionTargetByID(ctx, client, "proj1", "svcprimary", false) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -98,8 +138,8 @@ func TestResolveConnectionTargetByID(t *testing.T) { t.Errorf("expected primary target for a primary ID, got %+v", target) } - // Replica ID → replica target with parent credentials. - target, err = ResolveConnectionTargetByID(context.Background(), client, "proj1", "rep1234567") + // Replica set ID → replica target with the parent's credentials. + target, err = ResolveConnectionTargetByID(ctx, client, "proj1", "rep1234567", false) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -107,10 +147,40 @@ func TestResolveConnectionTargetByID(t *testing.T) { t.Errorf("expected replica target with parent credentials, got %+v", target) } - // Unknown ID → error. - if _, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "missing9999"); err == nil { - t.Error("expected an error for an unknown ID, got nil") + // Branch ID → branch target with its own credentials, when allowed. + target, err = ResolveConnectionTargetByID(ctx, client, "proj1", "br12345678", true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.Kind != TargetBranch || target.ID != "br12345678" || target.Credentials.ID != "br12345678" { + t.Errorf("expected branch target with its own credentials, got %+v", target) + } + + // Branch ID without branches allowed → not found, without naming branches. + _, err = ResolveConnectionTargetByID(ctx, client, "proj1", "br12345678", false) + if diff := cmp.Diff("service or read replica set 'br12345678' not found", errString(err)); diff != "" { + t.Errorf("gated branch error mismatch (-want +got):\n%s", diff) + } + + // A name resolves, but isn't an ID. + _, err = ResolveConnectionTargetByID(ctx, client, "proj1", "my-db", true) + if diff := cmp.Diff("service, branch, or read replica set 'my-db' not found", errString(err)); diff != "" { + t.Errorf("name match error mismatch (-want +got):\n%s", diff) + } + + // Unknown ID → not found. + _, err = ResolveConnectionTargetByID(ctx, client, "proj1", "missing999", true) + if diff := cmp.Diff("service, branch, or read replica set 'missing999' not found", errString(err)); diff != "" { + t.Errorf("unknown ID error mismatch (-want +got):\n%s", diff) + } +} + +// errString returns err's message, or "" for nil. +func errString(err error) string { + if err == nil { + return "" } + return err.Error() } func TestNewReplicaConnectionTarget(t *testing.T) { @@ -324,29 +394,13 @@ func TestReplicaPoolerWarning(t *testing.T) { } func TestResolveConnectionTargetByID_ReplicaParentFetchFails(t *testing.T) { - // The replica itself resolves, but its parent is absent from the server. - client := serviceTestClient(t, map[string]api.Service{"rep1234567": replicaService()}) + // The replica set itself resolves, but its parent is absent from the server. + replica := api.ReadReplicaSet{ID: "rep1234567", ServiceID: "svcprimary", Name: "reporting-replica"} + client := resolveTestClient(t, map[string]resolvedFixture{ + "rep1234567": {api.ResolveTypeREADREPLICASET, replica}, + }, nil) - if _, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "rep1234567"); err == nil { + if _, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "rep1234567", false); err == nil { t.Fatal("expected an error when the parent service can't be fetched, got nil") } } - -func TestResolveConnectionTargetByID_ReplicaWithoutParentID(t *testing.T) { - // A standby with no parent recorded has nowhere else to look for - // credentials, so it stands in as its own credential owner. - replica := replicaService() - replica.ForkedFrom.ServiceID = nil - client := serviceTestClient(t, map[string]api.Service{"rep1234567": replica}) - - target, err := ResolveConnectionTargetByID(context.Background(), client, "proj1", "rep1234567") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if target.Kind != TargetReadReplica { - t.Error("expected a replica target") - } - if target.Credentials.ID != "rep1234567" { - t.Errorf("expected the replica as its own credential owner, got %q", target.Credentials.ID) - } -} diff --git a/internal/mcp/db_extensions_list.go b/internal/mcp/db_extensions_list.go index 8fd36945..8245aecf 100644 --- a/internal/mcp/db_extensions_list.go +++ b/internal/mcp/db_extensions_list.go @@ -17,10 +17,10 @@ type DBExtensionsListInput struct { Role string `json:"role,omitempty"` } -func (DBExtensionsListInput) Schema() *jsonschema.Schema { +func (DBExtensionsListInput) Schema(experimental bool) *jsonschema.Schema { schema := util.Must(jsonschema.For[DBExtensionsListInput](nil)) - schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one lists the extensions on that read replica instead of the primary service." + schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one lists the extensions on that read replica instead of the primary service." + branchIDHelp(experimental, "lists the extensions on that branch") schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"} schema.Properties["service_id"].Pattern = "^[a-z0-9]{10}$" @@ -60,14 +60,14 @@ func (DBExtensionsListOutput) Schema() *jsonschema.Schema { return schema } -func newDBExtensionsListTool() *mcp.Tool { +func newDBExtensionsListTool(experimental bool) *mcp.Tool { return &mcp.Tool{ Name: toolDBExtensionsList, Title: "List Database Extensions", Description: `List the PostgreSQL extensions installed in a service database. Connects to a PostgreSQL/TimescaleDB service in Tiger Cloud and returns each installed extension's name, version, schema, and description. The connection is opened in immutable read-only mode.`, - InputSchema: DBExtensionsListInput{}.Schema(), + InputSchema: DBExtensionsListInput{}.Schema(experimental), OutputSchema: DBExtensionsListOutput{}.Schema(), Annotations: &mcp.ToolAnnotations{ ReadOnlyHint: true, @@ -84,14 +84,16 @@ func (s *Server) handleDBExtensionsList(ctx context.Context, req *mcp.CallToolRe return nil, DBExtensionsListOutput{}, err } - target, err := common.ResolveConnectionTargetByID(ctx, client, projectID, input.ServiceID) + // service_id may name a service, one of its read replicas, or (behind the + // experimental gate) a branch. + target, err := common.ResolveConnectionTargetByID(ctx, client, projectID, input.ServiceID, s.app.Experimental) if err != nil { return nil, DBExtensionsListOutput{}, err } extensions, err := common.FetchExtensions(ctx, cfg, target, input.Role) if err != nil { - return nil, DBExtensionsListOutput{}, handleDatabaseError(err) + return nil, DBExtensionsListOutput{}, handleDatabaseError(err, target) } return nil, DBExtensionsListOutput{Extensions: extensions}, nil diff --git a/internal/mcp/db_extensions_list_test.go b/internal/mcp/db_extensions_list_test.go index 65faca76..0b7477cb 100644 --- a/internal/mcp/db_extensions_list_test.go +++ b/internal/mcp/db_extensions_list_test.go @@ -38,7 +38,7 @@ func TestDBExtensionsListTool(t *testing.T) { setupGetWithStatus := func(status api.DeployStatus) func(m *mocks.MockClientWithResponsesInterface) { return func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "e6ue9697jf", sampleService(func(s *api.Service) { s.Status = status })) + expectResolve(m, "e6ue9697jf", nil, sampleService(func(s *api.Service) { s.Status = status })) } } @@ -53,11 +53,10 @@ func TestDBExtensionsListTool(t *testing.T) { primary := sampleService() primaryTarget := common.NewServiceConnectionTarget(primary) - replica := sampleReplica(func(s *api.Service) { s.Status = api.DeployStatusREADY }) + replica := sampleReplica(func(r *api.ReadReplicaSet) { r.Status = api.ReadReplicaSetStatusActive }) // A replica connects to its own endpoint with its primary's credentials. - replicaTarget := common.NewServiceConnectionTarget(replica) - replicaTarget.Kind = common.TargetReadReplica - replicaTarget.Credentials = common.ServiceCredentials(primary) + replicaTarget := common.NewReplicaConnectionTarget(replica, common.ServiceCredentials(primary)) + branch := sampleBranch() runToolTests(t, []toolTest{ { @@ -79,31 +78,26 @@ func TestDBExtensionsListTool(t *testing.T) { tool: toolDBExtensionsList, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). + m.EXPECT().ResolveRefWithResponse(validCtx, testProjectID, &api.ResolveRefParams{Ref: "e6ue9697jf"}). Return(nil, errors.New("connection refused")) }, - wantErr: "failed to fetch service details: connection refused", + wantErr: "failed to resolve service or read replica set 'e6ue9697jf': connection refused", }, { name: "service lookup API error", tool: toolDBExtensionsList, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). - Return(&api.GetServiceResponse{ - HTTPResponse: httpResponse(http.StatusNotFound), - JSON4XX: &api.ClientError{Message: new("service not found")}, - }, nil) + expectResolveStatus(m, "e6ue9697jf", nil, http.StatusNotFound, &api.ClientError{Message: new("resource not found")}) }, - wantErr: "service not found", + wantErr: "service or read replica set 'e6ue9697jf' not found", }, { name: "nil response body", tool: toolDBExtensionsList, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). - Return(&api.GetServiceResponse{HTTPResponse: httpResponse(http.StatusOK)}, nil) + expectResolveStatus(m, "e6ue9697jf", nil, http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -112,7 +106,7 @@ func TestDBExtensionsListTool(t *testing.T) { tool: toolDBExtensionsList, args: map[string]any{"service_id": "u8me885b93"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", replica) + expectResolve(m, "u8me885b93", nil, replica) m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). Return(&api.GetServiceResponse{ HTTPResponse: httpResponse(http.StatusNotFound), @@ -135,6 +129,28 @@ func TestDBExtensionsListTool(t *testing.T) { mock: setupGetWithStatus(api.DeployStatusQUEUED), wantErr: notReadyMsg, }, + { + // Branches are a preview, so without the gate a branch ID is + // reported as not found, without mentioning branches. + name: "branch not found without the experimental gate", + tool: toolDBExtensionsList, + args: map[string]any{"service_id": "br4nch1d9x"}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolve(m, "br4nch1d9x", nil, branch) + }, + wantErr: "service or read replica set 'br4nch1d9x' not found", + }, + { + // A branch is started with branch_start, not service_start. + name: "paused branch points at branch_start", + tool: toolDBExtensionsList, + args: map[string]any{"service_id": "br4nch1d9x"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolve(m, "br4nch1d9x", nil, sampleBranch(branchStatus(api.DeployStatusPAUSED))) + }, + wantErr: "branch is paused — start it with the branch_start tool", + }, { name: "ready service without an endpoint", tool: toolDBExtensionsList, @@ -180,10 +196,21 @@ func TestDBExtensionsListTool(t *testing.T) { args: map[string]any{"service_id": "u8me885b93"}, opts: []runOption{withFetchExtensions(replicaTarget, "tsdbadmin", extensions, nil)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", replica) + expectResolve(m, "u8me885b93", nil, replica) expectGetService(m, "e6ue9697jf", primary) }, wantOutput: map[string]any{"extensions": extensionsOutput}, }, + { + // A branch connects with its own credentials, not its parent's. + name: "returns the extensions of a branch", + tool: toolDBExtensionsList, + args: map[string]any{"service_id": "br4nch1d9x"}, + opts: []runOption{withExperimental(), withFetchExtensions(common.NewBranchConnectionTarget(branch), "tsdbadmin", extensions, nil)}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolve(m, "br4nch1d9x", nil, branch) + }, + wantOutput: map[string]any{"extensions": extensionsOutput}, + }, }) } diff --git a/internal/mcp/db_query.go b/internal/mcp/db_query.go index dcddd87e..a587dfde 100644 --- a/internal/mcp/db_query.go +++ b/internal/mcp/db_query.go @@ -34,10 +34,10 @@ type DBQueryInput struct { Pooled bool `json:"pooled,omitempty"` } -func (DBQueryInput) Schema() *jsonschema.Schema { +func (DBQueryInput) Schema(experimental bool) *jsonschema.Schema { schema := util.Must(jsonschema.For[DBQueryInput](nil)) - schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only) instead of the primary service." + schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one runs the query against that read replica (which is read-only) instead of the primary service." + branchIDHelp(experimental, "runs the query against that branch") schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"} schema.Properties["service_id"].Pattern = "^[a-z0-9]{10}$" @@ -112,7 +112,7 @@ func (DBQueryOutput) Schema() *jsonschema.Schema { return schema } -func newDBQueryTool() *mcp.Tool { +func newDBQueryTool(experimental bool) *mcp.Tool { return &mcp.Tool{ Name: toolDBQuery, Title: "Execute SQL Query", @@ -125,7 +125,7 @@ Multi-statement queries (semicolon-separated) are supported when no parameters a Process data in the database, not in your context: aggregate, filter, sort/limit, and join in SQL rather than fetching raw rows. WARNING: Can execute any SQL statement including INSERT, UPDATE, DELETE, and DDL commands. Always review queries before execution.`, - InputSchema: DBQueryInput{}.Schema(), + InputSchema: DBQueryInput{}.Schema(experimental), OutputSchema: DBQueryOutput{}.Schema(), Annotations: &mcp.ToolAnnotations{ ReadOnlyHint: false, @@ -162,8 +162,9 @@ func (s *Server) handleDBQuery(ctx context.Context, req *mcp.CallToolRequest, in slog.String("read_only", string(cfg.ReadOnly)), ) - // service_id may name a service or one of its read replicas. - target, err := common.ResolveConnectionTargetByID(ctx, client, projectID, input.ServiceID) + // service_id may name a service, one of its read replicas, or (behind the + // experimental gate) a branch. + target, err := common.ResolveConnectionTargetByID(ctx, client, projectID, input.ServiceID, s.app.Experimental) if err != nil { return nil, DBQueryOutput{}, err } @@ -192,7 +193,7 @@ func (s *Server) handleDBQuery(ctx context.Context, req *mcp.CallToolRequest, in MaxBytes: mcpMaxResponseBytes, }) if err != nil { - return nil, DBQueryOutput{}, handleDatabaseError(err) + return nil, DBQueryOutput{}, handleDatabaseError(err, target) } output := DBQueryOutput{ diff --git a/internal/mcp/db_query_test.go b/internal/mcp/db_query_test.go index 083c93b7..a2575515 100644 --- a/internal/mcp/db_query_test.go +++ b/internal/mcp/db_query_test.go @@ -41,7 +41,7 @@ func TestDBQueryTool(t *testing.T) { setupGetWithStatus := func(status api.DeployStatus) func(m *mocks.MockClientWithResponsesInterface) { return func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "e6ue9697jf", sampleService(func(s *api.Service) { s.Status = status })) + expectResolve(m, "e6ue9697jf", nil, sampleService(func(s *api.Service) { s.Status = status })) } } @@ -146,52 +146,79 @@ func TestDBQueryTool(t *testing.T) { tool: toolDBQuery, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). + m.EXPECT().ResolveRefWithResponse(validCtx, testProjectID, &api.ResolveRefParams{Ref: "e6ue9697jf"}). Return(nil, errors.New("connection refused")) }, - wantErr: "failed to fetch service details: connection refused", + wantErr: "failed to resolve service or read replica set 'e6ue9697jf': connection refused", }, { name: "service lookup API error", tool: toolDBQuery, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). - Return(&api.GetServiceResponse{ - HTTPResponse: httpResponse(http.StatusNotFound), - JSON4XX: &api.ClientError{Message: new("service not found")}, - }, nil) + expectResolveStatus(m, "e6ue9697jf", nil, http.StatusNotFound, &api.ClientError{Message: new("resource not found")}) }, - wantErr: "service not found", + wantErr: "service or read replica set 'e6ue9697jf' not found", }, { name: "nil response body", tool: toolDBQuery, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). - Return(&api.GetServiceResponse{HTTPResponse: httpResponse(http.StatusOK)}, nil) + expectResolveStatus(m, "e6ue9697jf", nil, http.StatusOK, nil) }, wantErr: "empty response from API", }, { - // Both services are fetched: the replica for its endpoint, the - // parent for its credentials. The replica's own status then stops it. + // The parent is fetched for its credentials, then the replica's own + // status stops it: a replica set that isn't active isn't ready. name: "read replica resolves its parent", tool: toolDBQuery, args: map[string]any{"service_id": "u8me885b93", "query": "SELECT 1"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", sampleReplica()) + expectResolve(m, "u8me885b93", nil, sampleReplica()) expectGetService(m, "e6ue9697jf", sampleService()) }, - wantErr: pausedMsg, + wantErr: notReadyMsg, + }, + { + // Branches are a preview, so without the gate a branch ID is + // reported as not found, without mentioning branches. + name: "branch not found without the experimental gate", + tool: toolDBQuery, + args: map[string]any{"service_id": "br4nch1d9x", "query": "SELECT 1"}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolve(m, "br4nch1d9x", nil, sampleBranch()) + }, + wantErr: "service or read replica set 'br4nch1d9x' not found", + }, + { + // A branch is started with branch_start, not service_start. + name: "paused branch points at branch_start", + tool: toolDBQuery, + args: map[string]any{"service_id": "br4nch1d9x", "query": "SELECT 1"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolve(m, "br4nch1d9x", nil, sampleBranch(branchStatus(api.DeployStatusPAUSED))) + }, + wantErr: "branch is paused — start it with the branch_start tool", + }, + { + name: "provisioning branch points at branch_get", + tool: toolDBQuery, + args: map[string]any{"service_id": "br4nch1d9x", "query": "SELECT 1"}, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectResolve(m, "br4nch1d9x", nil, sampleBranch(branchStatus(api.DeployStatusQUEUED))) + }, + wantErr: "branch is not ready — check its status with branch_get and try again", }, { name: "read replica parent lookup fails", tool: toolDBQuery, args: map[string]any{"service_id": "u8me885b93", "query": "SELECT 1"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", sampleReplica()) + expectResolve(m, "u8me885b93", nil, sampleReplica()) m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). Return(&api.GetServiceResponse{ HTTPResponse: httpResponse(http.StatusNotFound), @@ -226,7 +253,7 @@ func TestDBQueryTool(t *testing.T) { tool: toolDBQuery, args: map[string]any{"service_id": "e6ue9697jf", "query": "SELECT 1", "pooled": true}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "e6ue9697jf", sampleService(func(s *api.Service) { + expectResolve(m, "e6ue9697jf", nil, sampleService(func(s *api.Service) { s.Endpoint = &api.Endpoint{ Host: new("e6ue9697jf.project.tsdb.cloud.timescale.com"), Port: new(5432), @@ -272,7 +299,7 @@ func TestDBQueryTool(t *testing.T) { MaxBytes: mcpMaxResponseBytes, }, result, nil)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "e6ue9697jf", sampleService(func(s *api.Service) { + expectResolve(m, "e6ue9697jf", nil, sampleService(func(s *api.Service) { s.ConnectionPooler = &api.ConnectionPooler{ Endpoint: &api.Endpoint{ Host: new("e6ue9697jf.project.tsdb.cloud.timescale.com"), @@ -392,7 +419,7 @@ func TestDBQueryTool(t *testing.T) { MaxBytes: mcpMaxResponseBytes, }, result, nil)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", sampleReplica(func(s *api.Service) { s.Status = api.DeployStatusREADY })) + expectResolve(m, "u8me885b93", nil, sampleReplica(func(r *api.ReadReplicaSet) { r.Status = api.ReadReplicaSetStatusActive })) expectGetService(m, "e6ue9697jf", sampleService()) }, wantOutput: map[string]any{ diff --git a/internal/mcp/db_schema.go b/internal/mcp/db_schema.go index 9d12944b..39141b9d 100644 --- a/internal/mcp/db_schema.go +++ b/internal/mcp/db_schema.go @@ -23,10 +23,10 @@ type DBSchemaInput struct { Pooled bool `json:"pooled,omitempty"` } -func (DBSchemaInput) Schema() *jsonschema.Schema { +func (DBSchemaInput) Schema(experimental bool) *jsonschema.Schema { schema := util.Must(jsonschema.For[DBSchemaInput](nil)) - schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one introspects that read replica instead of the primary service." + schema.Properties["service_id"].Description = "Unique identifier of the service (10-character alphanumeric string). Use service_list to find service IDs. A read replica set ID is also accepted here — passing one introspects that read replica instead of the primary service." + branchIDHelp(experimental, "introspects that branch") schema.Properties["service_id"].Examples = []any{"e6ue9697jf", "u8me885b93"} schema.Properties["service_id"].Pattern = "^[a-z0-9]{10}$" @@ -69,7 +69,7 @@ func (DBSchemaOutput) Schema() *jsonschema.Schema { return schema } -func newDBSchemaTool() *mcp.Tool { +func newDBSchemaTool(experimental bool) *mcp.Tool { return &mcp.Tool{ Name: toolDBSchema, Title: "Show Database Schema", @@ -78,7 +78,7 @@ func newDBSchemaTool() *mcp.Tool { Connects to a PostgreSQL/TimescaleDB service in Tiger Cloud and returns its schema as readable text: tables (regular, partitioned, and foreign), views, materialized views, enum types, functions, procedures, indexes, triggers, and TimescaleDB hypertable and continuous aggregate metadata. Only objects the connecting role can access are returned. By default only user-facing schemas and objects are shown; view/routine definitions and object comments are omitted unless requested. The connection is opened in immutable read-only mode.`, - InputSchema: DBSchemaInput{}.Schema(), + InputSchema: DBSchemaInput{}.Schema(experimental), OutputSchema: DBSchemaOutput{}.Schema(), Annotations: &mcp.ToolAnnotations{ ReadOnlyHint: true, @@ -106,8 +106,9 @@ func (s *Server) handleDBSchema(ctx context.Context, req *mcp.CallToolRequest, i slog.Bool("pooled", input.Pooled), ) - // service_id may name a service or one of its read replicas. - target, err := common.ResolveConnectionTargetByID(ctx, client, projectID, input.ServiceID) + // service_id may name a service, one of its read replicas, or (behind the + // experimental gate) a branch. + target, err := common.ResolveConnectionTargetByID(ctx, client, projectID, input.ServiceID, s.app.Experimental) if err != nil { return nil, DBSchemaOutput{}, err } @@ -124,7 +125,7 @@ func (s *Server) handleDBSchema(ctx context.Context, req *mcp.CallToolRequest, i IncludeComments: input.Comments, }) if err != nil { - return nil, DBSchemaOutput{}, handleDatabaseError(err) + return nil, DBSchemaOutput{}, handleDatabaseError(err, target) } return nil, DBSchemaOutput{SchemaText: common.FormatSchema(schema), Warning: warning}, nil diff --git a/internal/mcp/db_schema_test.go b/internal/mcp/db_schema_test.go index db06c13c..b1a0b6d1 100644 --- a/internal/mcp/db_schema_test.go +++ b/internal/mcp/db_schema_test.go @@ -38,7 +38,7 @@ func TestDBSchemaTool(t *testing.T) { setupGetWithStatus := func(status api.DeployStatus) func(m *mocks.MockClientWithResponsesInterface) { return func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "e6ue9697jf", sampleService(func(s *api.Service) { s.Status = status })) + expectResolve(m, "e6ue9697jf", nil, sampleService(func(s *api.Service) { s.Status = status })) } } @@ -78,31 +78,26 @@ func TestDBSchemaTool(t *testing.T) { tool: toolDBSchema, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). + m.EXPECT().ResolveRefWithResponse(validCtx, testProjectID, &api.ResolveRefParams{Ref: "e6ue9697jf"}). Return(nil, errors.New("connection refused")) }, - wantErr: "failed to fetch service details: connection refused", + wantErr: "failed to resolve service or read replica set 'e6ue9697jf': connection refused", }, { name: "service lookup API error", tool: toolDBSchema, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). - Return(&api.GetServiceResponse{ - HTTPResponse: httpResponse(http.StatusNotFound), - JSON4XX: &api.ClientError{Message: new("service not found")}, - }, nil) + expectResolveStatus(m, "e6ue9697jf", nil, http.StatusNotFound, &api.ClientError{Message: new("resource not found")}) }, - wantErr: "service not found", + wantErr: "service or read replica set 'e6ue9697jf' not found", }, { name: "nil response body", tool: toolDBSchema, args: args, mock: func(m *mocks.MockClientWithResponsesInterface) { - m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). - Return(&api.GetServiceResponse{HTTPResponse: httpResponse(http.StatusOK)}, nil) + expectResolveStatus(m, "e6ue9697jf", nil, http.StatusOK, nil) }, wantErr: "empty response from API", }, @@ -128,23 +123,23 @@ func TestDBSchemaTool(t *testing.T) { wantErr: notReadyMsg, }, { - // Both services are fetched: the replica for its endpoint, the - // parent for its credentials. The replica's own status then stops it. + // The parent is fetched for its credentials, then the replica's own + // status stops it: a replica set that isn't active isn't ready. name: "read replica resolves its parent", tool: toolDBSchema, args: map[string]any{"service_id": "u8me885b93"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", sampleReplica()) + expectResolve(m, "u8me885b93", nil, sampleReplica()) expectGetService(m, "e6ue9697jf", sampleService()) }, - wantErr: pausedMsg, + wantErr: notReadyMsg, }, { name: "read replica parent lookup fails", tool: toolDBSchema, args: map[string]any{"service_id": "u8me885b93"}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", sampleReplica()) + expectResolve(m, "u8me885b93", nil, sampleReplica()) m.EXPECT().GetServiceWithResponse(validCtx, testProjectID, "e6ue9697jf"). Return(&api.GetServiceResponse{ HTTPResponse: httpResponse(http.StatusNotFound), @@ -165,7 +160,7 @@ func TestDBSchemaTool(t *testing.T) { tool: toolDBSchema, args: map[string]any{"service_id": "e6ue9697jf", "pooled": true}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "e6ue9697jf", sampleService(func(s *api.Service) { + expectResolve(m, "e6ue9697jf", nil, sampleService(func(s *api.Service) { s.Endpoint = &api.Endpoint{ Host: new("e6ue9697jf.project.tsdb.cloud.timescale.com"), Port: new(5432), @@ -221,7 +216,7 @@ func TestDBSchemaTool(t *testing.T) { args: map[string]any{"service_id": "u8me885b93", "pooled": true}, opts: []runOption{withFetchServiceSchema(common.FetchServiceSchemaArgs{Role: "tsdbadmin", Pooled: true}, schema, nil)}, mock: func(m *mocks.MockClientWithResponsesInterface) { - expectGetService(m, "u8me885b93", sampleReplica(func(s *api.Service) { s.Status = api.DeployStatusREADY })) + expectResolve(m, "u8me885b93", nil, sampleReplica(func(r *api.ReadReplicaSet) { r.Status = api.ReadReplicaSetStatusActive })) expectGetService(m, "e6ue9697jf", sampleService()) }, wantOutput: map[string]any{ diff --git a/internal/mcp/errors.go b/internal/mcp/errors.go index 7458ff24..8154a3e6 100644 --- a/internal/mcp/errors.go +++ b/internal/mcp/errors.go @@ -8,13 +8,21 @@ import ( ) // handleDatabaseError turns the readiness sentinels into guidance naming the -// tool that resolves them. Every other error passes through unchanged. -func handleDatabaseError(err error) error { +// tool that resolves them: a branch tool for a branch, a service tool for +// everything else. Every other error passes through unchanged. +func handleDatabaseError(err error, target *common.ConnectionTarget) error { + // The group also names the target, since the sentinels always say + // "service". + group := "service" + if target.Kind == common.TargetBranch { + group = "branch" + } + switch { case errors.Is(err, common.ErrPaused): - return fmt.Errorf("%w — start it with the service_start tool", common.ErrPaused) + return fmt.Errorf("%s is paused — start it with the %s_start tool", group, group) case errors.Is(err, common.ErrNotReady): - return fmt.Errorf("%w — check its status with service_get and try again", common.ErrNotReady) + return fmt.Errorf("%s is not ready — check its status with %s_get and try again", group, group) } return err } diff --git a/internal/mcp/main_test.go b/internal/mcp/main_test.go index e5d814d2..ad98ed25 100644 --- a/internal/mcp/main_test.go +++ b/internal/mcp/main_test.go @@ -469,25 +469,22 @@ func sampleService(overrides ...func(*api.Service)) api.Service { return svc } -// sampleReplica returns a paused standby read replica of sampleService, shaped -// the way GetService returns it when given a read replica set ID. A replica -// connects to its own endpoint but borrows the parent primary's credentials, -// so resolving one fetches both services. -func sampleReplica(overrides ...func(*api.Service)) api.Service { - svc := sampleService(func(s *api.Service) { - s.ServiceID = "u8me885b93" - s.Name = "replica-service" - s.Status = api.DeployStatusPAUSED - s.ForkedFrom = &api.ForkSpec{ - IsStandby: new(true), - ProjectID: new(testProjectID), - ServiceID: new("e6ue9697jf"), - } - }) +// sampleReplica returns a still-creating read replica set of sampleService, as +// the resolve endpoint returns it. A replica connects to its own endpoint but +// borrows the parent primary's credentials, so resolving one fetches the +// parent too. +func sampleReplica(overrides ...func(*api.ReadReplicaSet)) api.ReadReplicaSet { + replica := api.ReadReplicaSet{ + ID: "u8me885b93", + ServiceID: "e6ue9697jf", + Name: "replica-service", + Status: api.ReadReplicaSetStatusCreating, + Nodes: 1, + } for _, o := range overrides { - o(&svc) + o(&replica) } - return svc + return replica } // The readiness errors handleDatabaseError returns before a tool connects, diff --git a/internal/mcp/server.go b/internal/mcp/server.go index c8b66c38..fdd83447 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -210,7 +210,7 @@ func (s *Server) registerTools(ctx context.Context, mode config.ReadOnlyMode, ex s.registerBranchTools(mode, experimental) // Database operation tools - s.registerDatabaseTools(mode) + s.registerDatabaseTools(mode, experimental) // Submitting feedback mutates no service, so the tool is absent from // readOnlyGatedTools and stays registered in every read-only mode. @@ -233,7 +233,7 @@ func (s *Server) registerServiceTools(mode config.ReadOnlyMode, experimental boo addTool(s, mode, newServiceStopTool(), s.handleServiceStop) addTool(s, mode, newServiceResizeTool(experimental), s.handleServiceResize) addTool(s, mode, newServiceRenameTool(), s.handleServiceRename) - addTool(s, mode, newServiceDeleteTool(), s.handleServiceDelete) + addTool(s, mode, newServiceDeleteTool(experimental), s.handleServiceDelete) addTool(s, mode, newServiceLogsTool(), s.handleServiceLogs) addTool(s, mode, newServiceMetricsAvailableTool(), s.handleServiceMetricsAvailable) addTool(s, mode, newServiceMetricsDetailsTool(), s.handleServiceMetricsDetails) @@ -268,11 +268,11 @@ func (s *Server) registerBranchTools(mode config.ReadOnlyMode, experimental bool } // registerDatabaseTools registers database operation tools with comprehensive schemas and descriptions -func (s *Server) registerDatabaseTools(mode config.ReadOnlyMode) { - addTool(s, mode, newDBQueryTool(), s.handleDBQuery) +func (s *Server) registerDatabaseTools(mode config.ReadOnlyMode, experimental bool) { + addTool(s, mode, newDBQueryTool(experimental), s.handleDBQuery) - mcp.AddTool(s.mcpServer, newDBSchemaTool(), s.handleDBSchema) - mcp.AddTool(s.mcpServer, newDBExtensionsListTool(), s.handleDBExtensionsList) + mcp.AddTool(s.mcpServer, newDBSchemaTool(experimental), s.handleDBSchema) + mcp.AddTool(s.mcpServer, newDBExtensionsListTool(experimental), s.handleDBExtensionsList) } // analyticsMiddleware tracks analytics for all MCP requests diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go index eb157cba..d1c7edbe 100644 --- a/internal/mcp/server_test.go +++ b/internal/mcp/server_test.go @@ -40,7 +40,7 @@ func registeredToolNames(t *testing.T, readOnly config.ReadOnlyMode) []string { } s.registerServiceTools(readOnly, true) s.registerBranchTools(readOnly, true) - s.registerDatabaseTools(readOnly) + s.registerDatabaseTools(readOnly, true) clientTransport, serverTransport := mcp.NewInMemoryTransports() diff --git a/internal/mcp/service_delete.go b/internal/mcp/service_delete.go index 9b5db845..a57b8cdf 100644 --- a/internal/mcp/service_delete.go +++ b/internal/mcp/service_delete.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "net/http" + "strings" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -48,13 +49,20 @@ func (ServiceDeleteOutput) Schema() *jsonschema.Schema { return schema } -func newServiceDeleteTool() *mcp.Tool { - return &mcp.Tool{ - Name: toolServiceDelete, - Title: "Delete Database Service", - Description: `Delete a database service permanently. +func newServiceDeleteTool(experimental bool) *mcp.Tool { + description := "Delete a database service permanently.\n\n" + + "This operation is irreversible: the service and all of its data are destroyed. " + // Branches are a preview, so only the experimental description mentions + // them. The PROD prompt lists them regardless: they're deleted either way. + if experimental { + description += "Deleting a service also deletes all of its branches. " + } + description += `Deleting a service tagged PROD automatically prompts the user, via an elicitation request through the MCP client, to confirm the deletion before it proceeds, so agents don't need to ask the user for confirmation themselves; if the client cannot prompt, the deletion is refused and the user must run 'tiger service delete' from the CLI instead.` -This operation is irreversible: the service and all of its data are destroyed. Deleting a service tagged PROD automatically prompts the user, via an elicitation request through the MCP client, to confirm the deletion before it proceeds, so agents don't need to ask the user for confirmation themselves; if the client cannot prompt, the deletion is refused and the user must run 'tiger service delete' from the CLI instead.`, + return &mcp.Tool{ + Name: toolServiceDelete, + Title: "Delete Database Service", + Description: description, InputSchema: ServiceDeleteInput{}.Schema(), OutputSchema: ServiceDeleteOutput{}.Schema(), Annotations: &mcp.ToolAnnotations{ @@ -98,10 +106,14 @@ func (s *Server) handleServiceDelete(ctx context.Context, req *mcp.CallToolReque return nil, ServiceDeleteOutput{}, err } + // Listed on every run, PROD's prompting run included, so both the prompt + // and the result can name them. + branches, branchesErr := s.listBranchesToDelete(ctx, client, projectID, *service) + if tag == api.EnvironmentTagPROD { answer, answered := req.Params.InputResponses[deleteConfirmationKey] if !answered { - result, err := promptProdDelete(req, *service) + result, err := promptProdDelete(req, *service, branchesToDeleteMessage(branches, branchesErr)) return result, ServiceDeleteOutput{}, err } if !serviceIDConfirmed(answer, input.ServiceID) { @@ -125,24 +137,64 @@ func (s *Server) handleServiceDelete(ctx context.Context, req *mcp.CallToolReque return nil, ServiceDeleteOutput{}, common.ExitWithErrorFromStatusCode(resp.StatusCode(), resp.JSON4XX) } + message := fmt.Sprintf("Service %q has been deleted.", input.ServiceID) + if len(branches) > 0 { + message = fmt.Sprintf("Service %q has been deleted, along with its %s: %s.", input.ServiceID, common.FormatBranchCount(len(branches)), branchLabels(branches)) + } return nil, ServiceDeleteOutput{ ServiceID: input.ServiceID, Deleted: true, - Message: fmt.Sprintf("Service %q has been deleted.", input.ServiceID), + Message: message, }, nil } // promptProdDelete returns the input-required result that asks the user to -// confirm deleting a PROD service. It refuses outright when the client can't -// show the prompt: silently deleting a production service without a human in -// the loop is the one outcome this tool must never produce. -func promptProdDelete(req *mcp.CallToolRequest, service api.Service) (*mcp.CallToolResult, error) { +// confirm deleting a PROD service, with branchesNote (from +// branchesToDeleteMessage) appended to the question. It refuses outright when +// the client can't show the prompt: silently deleting a production service +// without a human in the loop is the one outcome this tool must never produce. +func promptProdDelete(req *mcp.CallToolRequest, service api.Service, branchesNote string) (*mcp.CallToolResult, error) { if !clientSupportsFormElicitation(req) { return nil, fmt.Errorf("deleting service %s requires the user's confirmation because it is tagged PROD, but this MCP client does not support elicitation; ask the user to run 'tiger service delete %s' instead", service.ServiceID, service.ServiceID) } - return serviceIDConfirmationRequest( - deleteConfirmationKey, - fmt.Sprintf("Delete PRODUCTION service %q (%s)? This permanently destroys the service and all of its data, and cannot be undone.", service.Name, service.ServiceID), - service.ServiceID, - ), nil + message := fmt.Sprintf("Delete PRODUCTION service %q (%s)? This permanently destroys the service and all of its data, and cannot be undone.", service.Name, service.ServiceID) + return serviceIDConfirmationRequest(deleteConfirmationKey, message+branchesNote, service.ServiceID), nil +} + +// listBranchesToDelete returns the branches that deleting service takes with +// it. The API deletes them without asking, so this isn't gated on the +// experimental env var: whoever has branches gets told. A failed listing is +// logged and returned rather than failing the call, so the delete can still go +// ahead. +func (s *Server) listBranchesToDelete(ctx context.Context, client api.ClientWithResponsesInterface, projectID string, service api.Service) ([]api.Branch, error) { + if !common.CanHaveBranches(service) { + return nil, nil + } + branches, err := common.ListBranches(ctx, client, projectID, service.ServiceID) + if err != nil { + s.logger.Error("MCP: Failed to list branches", slog.Any("error", err)) + } + return branches, err +} + +// branchesToDeleteMessage is the sentence the PROD prompt appends about the +// branches deleted with the service: their names, a warning in their place if +// listing them failed, or "" if there are none. +func branchesToDeleteMessage(branches []api.Branch, err error) string { + if err != nil { + return " Any branches of the service will also be deleted." + } + if len(branches) == 0 { + return "" + } + return fmt.Sprintf(" Its %s will also be deleted: %s.", common.FormatBranchCount(len(branches)), branchLabels(branches)) +} + +// branchLabels lists branches by name and ID, e.g. `"a" (id1), "b" (id2)`. +func branchLabels(branches []api.Branch) string { + labels := make([]string, len(branches)) + for i, branch := range branches { + labels[i] = fmt.Sprintf("%q (%s)", branch.Name, branch.ID) + } + return strings.Join(labels, ", ") } diff --git a/internal/mcp/service_delete_test.go b/internal/mcp/service_delete_test.go index 299da832..09e22020 100644 --- a/internal/mcp/service_delete_test.go +++ b/internal/mcp/service_delete_test.go @@ -1,7 +1,9 @@ package mcp import ( + "errors" "net/http" + "strings" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -54,6 +56,45 @@ func TestServiceDeleteTool(t *testing.T) { "required": []any{"service_id"}, }, } + // promptWith is prompt with extra appended to its message, for a fluid + // service whose branches the prompt names. + promptWith := func(extra string) *mcp.ElicitParams { + p := *prompt + p.Message += extra + return &p + } + // expectFluidService registers fetching a fluid service tagged tag, the + // kind that can have branches, on each of calls handler runs. + expectFluidService := func(tag string, calls int) func(*mocks.MockClientWithResponsesInterface) { + return func(m *mocks.MockClientWithResponsesInterface) { + expectGetService(m, "e6ue9697jf", sampleService(func(s *api.Service) { + s.Metadata = &api.ServiceMetadata{Environment: &tag} + s.Platform = new(api.ServicePlatformFLUID) + })).Times(calls) + } + } + // expectBranches registers listing the service's branches on each of + // calls handler runs. + expectBranches := func(m *mocks.MockClientWithResponsesInterface, calls int, branches ...api.Branch) { + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "e6ue9697jf", nil). + Return(&api.GetBranchesResponse{ + HTTPResponse: httpResponse(http.StatusOK), + JSON200: &branches, + }, nil).Times(calls) + } + otherBranch := sampleBranch(func(b *api.Branch) { + b.ID = "x7k2m9p4qa" + b.Name = "other-branch" + }) + // deletedWith is a successful deletion's result, with the given message. + deletedWith := func(message string) map[string]any { + return map[string]any{ + "service_id": "e6ue9697jf", + "deleted": true, + "message": message, + } + } + // confirm answers the prompt by typing id back. confirm := func(id string) runOption { return withElicitation(&mcp.ElicitResult{Action: "accept", Content: map[string]any{"service_id": id}}) @@ -129,6 +170,77 @@ func TestServiceDeleteTool(t *testing.T) { wantPrompt: prompt, wantOutput: deleted, }, + { + // The API deletes a service's branches along with it, so the + // prompt and the result name them even without the experimental + // gate. Both handler runs list them. + name: "PROD prompt and result name the branches deleted with the service", + tool: toolServiceDelete, + args: args, + opts: []runOption{confirm("e6ue9697jf")}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectFluidService("PROD", 2)(m) + expectBranches(m, 2, sampleBranch(), otherBranch) + expectDelete(m) + }, + wantPrompt: promptWith(` Its 2 branches will also be deleted: "test-branch" (br4nch1d9x), "other-branch" (x7k2m9p4qa).`), + wantOutput: deletedWith(`Service "e6ue9697jf" has been deleted, along with its 2 branches: "test-branch" (br4nch1d9x), "other-branch" (x7k2m9p4qa).`), + }, + { + name: "PROD prompt names a single branch", + tool: toolServiceDelete, + args: args, + opts: []runOption{confirm("e6ue9697jf")}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectFluidService("PROD", 2)(m) + expectBranches(m, 2, sampleBranch()) + expectDelete(m) + }, + wantPrompt: promptWith(` Its 1 branch will also be deleted: "test-branch" (br4nch1d9x).`), + wantOutput: deletedWith(`Service "e6ue9697jf" has been deleted, along with its 1 branch: "test-branch" (br4nch1d9x).`), + }, + { + name: "PROD prompt for a fluid service without branches names none", + tool: toolServiceDelete, + args: args, + opts: []runOption{confirm("e6ue9697jf")}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectFluidService("PROD", 2)(m) + expectBranches(m, 2) + expectDelete(m) + }, + wantPrompt: prompt, + wantOutput: deleted, + }, + { + // The listing only informs the decision, so a failure still + // prompts, with a warning in place of the names. + name: "PROD prompt when the branch listing fails", + tool: toolServiceDelete, + args: args, + opts: []runOption{confirm("e6ue9697jf")}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectFluidService("PROD", 2)(m) + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "e6ue9697jf", nil). + Return(nil, errors.New("connection refused")).Times(2) + expectDelete(m) + }, + wantPrompt: promptWith(" Any branches of the service will also be deleted."), + wantOutput: deleted, + }, + { + // DEV deletions don't prompt, but the result still names the + // branches deleted along with the service. + name: "deletes fluid DEV service and names its branches", + tool: toolServiceDelete, + args: args, + mock: func(m *mocks.MockClientWithResponsesInterface) { + expectFluidService("DEV", 1)(m) + expectBranches(m, 1, sampleBranch(), otherBranch) + expectDelete(m) + }, + wantOutput: deletedWith(`Service "e6ue9697jf" has been deleted, along with its 2 branches: "test-branch" (br4nch1d9x), "other-branch" (x7k2m9p4qa).`), + }, { name: "PROD deletion cancelled when the typed ID does not match", tool: toolServiceDelete, @@ -199,3 +311,16 @@ func TestServiceDeleteTool(t *testing.T) { }, }) } + +// TestServiceDeleteToolDescription covers the description's experimental +// gate at the helper level: the harness calls tools but never lists them, so +// a description isn't reachable through runTool. +func TestServiceDeleteToolDescription(t *testing.T) { + const branchNote = "Deleting a service also deletes all of its branches." + if got := newServiceDeleteTool(false).Description; strings.Contains(got, "branch") { + t.Errorf("description must not mention branches without the gate, got:\n%s", got) + } + if got := newServiceDeleteTool(true).Description; !strings.Contains(got, branchNote) { + t.Errorf("description must contain %q behind the gate, got:\n%s", branchNote, got) + } +} diff --git a/internal/mcp/service_get.go b/internal/mcp/service_get.go index 06a6c1de..6aa6d328 100644 --- a/internal/mcp/service_get.go +++ b/internal/mcp/service_get.go @@ -84,6 +84,16 @@ func (s *Server) handleServiceGet(ctx context.Context, req *mcp.CallToolRequest, Service: s.convertToServiceDetail(cfg, *resp.JSON200, input.WithPassword), } + // Branches are a preview, so they're counted only behind the gate. The + // rest of the output doesn't depend on the count. + if s.app.Experimental && common.CanHaveBranches(*resp.JSON200) { + if count, err := common.CountBranches(ctx, client, projectID, input.ServiceID); err != nil { + s.logger.Error("MCP: Failed to count branches", slog.Any("error", err)) + } else { + output.Service.BranchCount = &count + } + } + // Check if password was requested but not available if input.WithPassword && output.Service.Password == "" { return nil, ServiceGetOutput{}, fmt.Errorf("requested password but password not available") diff --git a/internal/mcp/service_get_test.go b/internal/mcp/service_get_test.go index 27aa70f1..3541974f 100644 --- a/internal/mcp/service_get_test.go +++ b/internal/mcp/service_get_test.go @@ -47,6 +47,11 @@ func TestServiceGetTool(t *testing.T) { s.Created = created s.Endpoint = endpoint }) + fluidService := sampleService(func(s *api.Service) { + s.Created = created + s.Endpoint = endpoint + s.Platform = new(api.ServicePlatformFLUID) + }) setupGet := func(svc api.Service) func(m *mocks.MockClientWithResponsesInterface) { return func(m *mocks.MockClientWithResponsesInterface) { @@ -170,18 +175,27 @@ func TestServiceGetTool(t *testing.T) { }, }, { - // The platform and IOPS are preview fields: the case above carries - // them and drops them, while this one returns them. - name: "experimental returns the platform and IOPS", + // The platform, IOPS, and branch count are preview fields: the case + // above carries a platform and IOPS and drops them, while this one + // returns all three. + name: "experimental returns the platform, IOPS, and branch count", tool: toolServiceGet, args: args, opts: []runOption{withExperimental()}, - mock: setupGet(sampleService(func(s *api.Service) { - s.Created = created - s.Endpoint = endpoint - s.Platform = new(api.ServicePlatformFLUID) - s.Resources = []api.Resource{{Spec: &api.ResourceSpec{CPUMillis: new(2000), MemoryGbs: new(16), Iops: new(12000)}}} - })), + mock: func(m *mocks.MockClientWithResponsesInterface) { + setupGet(sampleService(func(s *api.Service) { + s.Created = created + s.Endpoint = endpoint + s.Platform = new(api.ServicePlatformFLUID) + s.Resources = []api.Resource{{Spec: &api.ResourceSpec{CPUMillis: new(2000), MemoryGbs: new(16), Iops: new(12000)}}} + }))(m) + branches := []api.Branch{sampleBranch(), sampleBranch()} + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "e6ue9697jf", nil). + Return(&api.GetBranchesResponse{ + HTTPResponse: httpResponse(http.StatusOK), + JSON200: &branches, + }, nil) + }, wantOutput: map[string]any{ "service": map[string]any{ "id": "e6ue9697jf", @@ -194,6 +208,59 @@ func TestServiceGetTool(t *testing.T) { "environment": "DEV", "resources": map[string]any{"cpu": "2 cores", "memory": "16 GB", "iops": float64(12000)}, "replicas": float64(0), + "branch_count": float64(2), + "direct_endpoint": "e6ue9697jf.project.tsdb.cloud.timescale.com:5432", + "connection_string": "postgresql://tsdbadmin@e6ue9697jf.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require", + }, + }, + }, + { + // Only fluid services can be branched, so a standard one isn't + // counted. The mock fails the test on any branch listing. + name: "experimental standard service lists no branches", + tool: toolServiceGet, + args: args, + opts: []runOption{withExperimental()}, + mock: setupGet(noPasswordService), + wantOutput: map[string]any{ + "service": map[string]any{ + "id": "e6ue9697jf", + "name": "test-service", + "status": "READY", + "platform": "STANDARD", + "type": "TIMESCALEDB", + "region": "us-east-1", + "created": "2025-01-15T09:30:00Z", + "environment": "DEV", + "replicas": float64(0), + "direct_endpoint": "e6ue9697jf.project.tsdb.cloud.timescale.com:5432", + "connection_string": "postgresql://tsdbadmin@e6ue9697jf.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require", + }, + }, + }, + { + // The count is the only thing the branch listing adds, so the + // service is still returned without it. + name: "branch count fails", + tool: toolServiceGet, + args: args, + opts: []runOption{withExperimental()}, + mock: func(m *mocks.MockClientWithResponsesInterface) { + setupGet(fluidService)(m) + m.EXPECT().GetBranchesWithResponse(validCtx, testProjectID, "e6ue9697jf", nil). + Return(nil, errors.New("connection refused")) + }, + wantOutput: map[string]any{ + "service": map[string]any{ + "id": "e6ue9697jf", + "name": "test-service", + "status": "READY", + "platform": "FLUID", + "type": "TIMESCALEDB", + "region": "us-east-1", + "created": "2025-01-15T09:30:00Z", + "environment": "DEV", + "replicas": float64(0), "direct_endpoint": "e6ue9697jf.project.tsdb.cloud.timescale.com:5432", "connection_string": "postgresql://tsdbadmin@e6ue9697jf.project.tsdb.cloud.timescale.com:5432/tsdb?sslmode=require", }, diff --git a/internal/mcp/utils.go b/internal/mcp/utils.go index bb10a324..139bed12 100644 --- a/internal/mcp/utils.go +++ b/internal/mcp/utils.go @@ -94,6 +94,16 @@ func setPlatformSchemaProperties(schema *jsonschema.Schema, experimental bool) { schema.Properties["platform"].Enum = []any{api.ServicePlatformSTANDARD, api.ServicePlatformFLUID} } +// setBranchCountSchemaProperties describes a service's branch count, or +// removes it outside the experimental gate, since branches are a preview. +func setBranchCountSchemaProperties(schema *jsonschema.Schema, experimental bool) { + if !experimental { + delete(schema.Properties, "branch_count") + return + } + schema.Properties["branch_count"].Description = "Number of branches of the service (only returned by service_get, for fluid services)" +} + // setIOPSSchemaProperties sets a tool's iops input property. IOPS are a // preview parameter, so without the experimental gate the property is removed, // and the schema rejects it as an unknown property. defaultDescription, if @@ -181,6 +191,7 @@ type ServiceDetail struct { Environment string `json:"environment"` Resources *ResourceInfo `json:"resources,omitempty"` Replicas int `json:"replicas"` + BranchCount *int `json:"branch_count,omitempty"` DirectEndpoint string `json:"direct_endpoint,omitempty"` PoolerEndpoint string `json:"pooler_endpoint,omitempty"` Password string `json:"password,omitempty"` @@ -204,6 +215,7 @@ func (ServiceDetail) Schema(experimental bool) *jsonschema.Schema { setResourceInfoSchemaProperties(schema.Properties["resources"], experimental) schema.Properties["replicas"].Description = "Number of HA replicas (0=single node/no HA, 1+=HA enabled)" + setBranchCountSchemaProperties(schema, experimental) schema.Properties["direct_endpoint"].Description = "Direct database connection endpoint" schema.Properties["pooler_endpoint"].Description = "Connection pooler endpoint" schema.Properties["password"].Description = "Password for tsdbadmin user (only included if with_password=true)" @@ -276,6 +288,16 @@ func (s *Server) convertToServiceDetail(cfg *config.Config, service api.Service, return detail } +// branchIDHelp is the sentence a db tool's service_id description gains for +// branches, saying what passing a branch ID does. Branches are a preview, so +// it is empty without the experimental gate. +func branchIDHelp(experimental bool, does string) string { + if !experimental { + return "" + } + return " A branch ID is also accepted — passing one " + does + " instead. Use branch_list to find branch IDs." +} + // setBranchIDSchemaProperties sets common branch_id schema properties. Like // service_id, branch_id is IDs-only (see setServiceIDSchemaProperties), which // resolveBranchByID enforces.