diff --git a/README.md b/README.md index d82fa9130..1910bde9d 100644 --- a/README.md +++ b/README.md @@ -461,7 +461,7 @@ Sixteen domain tools (`basecamp_projects`, `basecamp_todos`, `basecamp_cards`, account. Each tool takes `{"action": "...", "params": {...}}` and serves per-action schemas through its `describe` action. -Two composite helpers ride on that surface, shared with basecamp-mcp-server +Composite helpers ride on that surface, shared with basecamp-mcp-server through basecamp-sdk so both servers answer the same way. `basecamp_recordings summarize` resolves the pointer an account event feed row carries — bucket id, recording id, and the event or recording type — @@ -471,7 +471,10 @@ it carries no method and no path, where every model operation carries both, and its summary opens with "Synthetic:". `basecamp_messages create_comment` is an ordinary endpoint with a synthetic parameter: `mentions`, a list of person ids expanded client-side into the attachment markup Basecamp reads as -a mention. +a mention. `basecamp_recordings move` and `copy` file a recording into +another container, in its project or another one, as the web's Move… and +Copy… dialogs do: they wait for Basecamp to finish and return the new +recording, and a failure says whether trying again is safe. ```bash basecamp mcp --read-only # serve only read-only actions diff --git a/go.mod b/go.mod index 3496ffbd4..6aefec586 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( charm.land/bubbles/v2 v2.2.1 charm.land/bubbletea/v2 v2.1.0 charm.land/lipgloss/v2 v2.0.6 - github.com/basecamp/basecamp-sdk/go v0.31.0 + github.com/basecamp/basecamp-sdk/go v0.32.0 github.com/basecamp/cli v0.2.2-0.20261023171439-c9bce8206df9 github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d github.com/basecamp/surfguard/go v0.1.0 diff --git a/go.sum b/go.sum index 26d423ea9..ec9182111 100644 --- a/go.sum +++ b/go.sum @@ -89,8 +89,8 @@ github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuP github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= github.com/basecamp/actioncable-go v1.2.0 h1:t0MxlfMTcmaGXZ5ztgKRs7ZiY1MwXtBRwSSSAh1ofVg= github.com/basecamp/actioncable-go v1.2.0/go.mod h1:ezaV5z1GXQAsqyejqTs6wCFl2D8Wj+COLQkHc/kwoRs= -github.com/basecamp/basecamp-sdk/go v0.31.0 h1:+gAcD0hMiEt4VWP4VOSOr/U3crjVa6dfT/NjmPN2JDo= -github.com/basecamp/basecamp-sdk/go v0.31.0/go.mod h1:mUm/MQGMyg1E1cddyxdG7tQHiq0NsYee2zUeAUT5VjU= +github.com/basecamp/basecamp-sdk/go v0.32.0 h1:HWRxJGhlaxzh7f/2+cxax664dN5BgjfM+Our3rEWuZU= +github.com/basecamp/basecamp-sdk/go v0.32.0/go.mod h1:mUm/MQGMyg1E1cddyxdG7tQHiq0NsYee2zUeAUT5VjU= github.com/basecamp/cli v0.2.2-0.20261023171439-c9bce8206df9 h1:3NfGs+zx23Tp6cf+97+UewrJHlzZMLzmvEXOmOJyQAk= github.com/basecamp/cli v0.2.2-0.20261023171439-c9bce8206df9/go.mod h1:iTBTaWvsPEFIcZfkxQHEfISyJ6sZ7036K6bNx0RY3EE= github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d h1:zEQVGq1x1nhKMZ2TudFAcSJ32CHT8richI1vQakIKz4= diff --git a/internal/mcpserver/catalog_test.go b/internal/mcpserver/catalog_test.go index f1f2975ad..b3d9f8926 100644 --- a/internal/mcpserver/catalog_test.go +++ b/internal/mcpserver/catalog_test.go @@ -51,7 +51,9 @@ func TestCatalogClaimsEveryTag(t *testing.T) { // upload). The stream-ticket mint could ride it and is dropped anyway: its // result is a replayable bearer and a URL embedding it, the dispatcher // returns a result verbatim into a model transcript with no reveal opt-in, -// and nothing on this surface could open the WebSocket it is for. +// and nothing on this surface could open the WebSocket it is for. The +// start-only move and copy are dropped at load instead, where the waiting +// composites take their place. func TestCatalogExcludesUnservedOperations(t *testing.T) { cat := loadForTest(t) excluded := map[string]bool{ @@ -61,6 +63,9 @@ func TestCatalogExcludesUnservedOperations(t *testing.T) { "UpdateProjectWallpaper": true, "UpdateProjectLogo": true, "CreateStreamTicket": true, + // Replaced by the waiting recordings.move and recordings.copy. + "MoveRecording": true, + "CopyRecording": true, } model, composite := 0, 0 for _, d := range cat.Domains { @@ -73,8 +78,8 @@ func TestCatalogExcludesUnservedOperations(t *testing.T) { assert.False(t, excluded[op.ID], "operation %q should be excluded from the vendored model", op.ID) } } - assert.Equal(t, 286, model, "served model operation count") - assert.Equal(t, 1, composite, "served composite operation count") + assert.Equal(t, 284, model, "served model operation count") + assert.Equal(t, 3, composite, "served composite operation count") } // isComposite reports the SDK compositions this package serves itself: no @@ -105,7 +110,7 @@ func TestCatalogMarksCompositesSynthetic(t *testing.T) { assert.NotEmpty(t, op.Doc, "composite %q must document what it composes", op.ID) } } - assert.Equal(t, map[string]bool{"recordings.summarize": true}, seen) + assert.Equal(t, map[string]bool{"recordings.summarize": true, "recordings.move": true, "recordings.copy": true}, seen) } // TestCatalogDeclaresMentionsOnCreateComment pins the other half of the diff --git a/internal/mcpserver/composite.go b/internal/mcpserver/composite.go index 8d1a5c47e..62b62bf73 100644 --- a/internal/mcpserver/composite.go +++ b/internal/mcpserver/composite.go @@ -51,7 +51,7 @@ const ( mentionsParam = "mentions" syntheticSummaryTag = "Synthetic: " recordingsDomainTag = "Recordings" - recordingsDomainDesc = "Recordings: the compact projection of one recording, resolved from the pointer an account event feed row carries." + recordingsDomainDesc = "Recordings: the compact projection of one recording, resolved from the pointer an account event feed row carries; and moving or copying a recording into another container or project, waiting for Basecamp to finish and returning the new recording." ) // maxExactJSONInteger is the largest integer a JSON number carries without @@ -84,6 +84,8 @@ type compositeHandler func(ctx context.Context, api API, op *catalog.Operation, // dispatcher holds when a call arrives. var compositeHandlers = map[string]compositeHandler{ recordingsDomainKey + "." + summarizeAction: handleSummarize, + recordingsDomainKey + "." + moveAction: handleFiling(basecamp.FilingMove), + recordingsDomainKey + "." + copyAction: handleFiling(basecamp.FilingCopy), } // recordingsDomain is the hand-built domain carrying the composite reads. @@ -96,6 +98,8 @@ func recordingsDomain() *catalog.Domain { Tool: recordingsToolName, Blurb: recordingsDomainDesc, Operations: []*catalog.Operation{ + filingOperation(basecamp.FilingCopy), + filingOperation(basecamp.FilingMove), { ID: "SummarizeRecording", Action: summarizeAction, @@ -131,7 +135,8 @@ func recordingsDomain() *catalog.Domain { } // installComposites adds the composite surface to a loaded catalog: the -// recordings domain, and the mentions parameter on messages.create_comment. +// recordings domain, which takes over the model's move, copy and filing +// operations, and the mentions parameter on messages.create_comment. // It runs after rescopeToAccount, which walks model paths the composite // operations deliberately do not have. func installComposites(cat *catalog.Catalog) error { @@ -140,7 +145,11 @@ func installComposites(cat *catalog.Catalog) error { return fmt.Errorf("the model already serves a %q domain; the composite domain would shadow it", recordingsDomainKey) } } - cat.Domains = append(cat.Domains, recordingsDomain()) + recordings := recordingsDomain() + if err := installFilingOperations(cat, recordings); err != nil { + return err + } + cat.Domains = append(cat.Domains, recordings) return installMentionsParam(cat) } @@ -208,16 +217,8 @@ func handleSummarize(ctx context.Context, api API, op *catalog.Operation, params func summarizeRef(op *catalog.Operation, params map[string]any) (basecamp.RecordingRef, error) { var ref basecamp.RecordingRef - // What is accepted is what is advertised: the same body schema describe - // serves, read here rather than restated, so the two cannot drift. - known, _ := op.Body["properties"].(map[string]any) - if len(known) == 0 { - return ref, fmt.Errorf("internal error: action %q declares no parameters", op.Action) - } - for name := range params { - if _, ok := known[name]; !ok { - return ref, fmt.Errorf("unknown parameter %q for action %q (describe the action for its schema)", name, op.Action) - } + if err := refuseUnknownParams(op, params); err != nil { + return ref, err } var err error @@ -239,6 +240,22 @@ func summarizeRef(op *catalog.Operation, params map[string]any) (basecamp.Record return ref, nil } +// refuseUnknownParams holds a composite's parameters to what it advertises: +// the same body schema describe serves, read here rather than restated, so +// the two cannot drift. +func refuseUnknownParams(op *catalog.Operation, params map[string]any) error { + known, _ := op.Body["properties"].(map[string]any) + if len(known) == 0 { + return fmt.Errorf("internal error: action %q declares no parameters", op.Action) + } + for name := range params { + if _, ok := known[name]; !ok { + return fmt.Errorf("unknown parameter %q for action %q (describe the action for its schema)", name, op.Action) + } + } + return nil +} + // errNoTypeGiven is a pointer with neither type. The SDK reaches the same // verdict for a pointer it cannot route, so this wraps that sentinel and is // reported under the same identity: a caller branching on the error's type diff --git a/internal/mcpserver/dispatch.go b/internal/mcpserver/dispatch.go index 3383ec754..06e0c755f 100644 --- a/internal/mcpserver/dispatch.go +++ b/internal/mcpserver/dispatch.go @@ -25,8 +25,8 @@ import ( // // The four verbs serve every model operation but the feed's two poll lanes. // The recordings and comments services serve the composite actions (see -// composite.go), which are SDK compositions rather than single requests and so -// cannot be assembled from a method and a path. The event feed service serves +// composite.go and filing.go), which are SDK compositions rather than single +// requests and so cannot be assembled from a method and a path. The event feed service serves // poll_events and poll_inbox, whose refusals carry the data a consumer resumes // from and whose bodies the verbs above throw away (see feed.go). // @@ -44,6 +44,23 @@ type API interface { Recordings() *basecamp.RecordingsService Comments() *basecamp.CommentsService EventFeed() *basecamp.EventFeedService + + // The typed reads and the project dock that move and copy resolve a + // recording and its destination through (see filing.go). + Projects() *basecamp.ProjectsService + Cards() *basecamp.CardsService + CardColumns() *basecamp.CardColumnsService + Messages() *basecamp.MessagesService + Documents() *basecamp.DocumentsService + Uploads() *basecamp.UploadsService + CloudFiles() *basecamp.CloudFilesService + GoogleDocuments() *basecamp.GoogleDocumentsService + Vaults() *basecamp.VaultsService + Schedules() *basecamp.SchedulesService + Checkins() *basecamp.CheckinsService + Forwards() *basecamp.ForwardsService + Todolists() *basecamp.TodolistsService + Todos() *basecamp.TodosService } // The CLI hands its account-scoped client straight to New. diff --git a/internal/mcpserver/filing.go b/internal/mcpserver/filing.go new file mode 100644 index 000000000..f063ac359 --- /dev/null +++ b/internal/mcpserver/filing.go @@ -0,0 +1,540 @@ +package mcpserver + +import ( + "bytes" + "cmp" + "context" + "encoding/json" + "errors" + "fmt" + "maps" + "slices" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/basecamp/mcp/catalog" + "github.com/basecamp/mcp/gateway" +) + +// Filing actions: recordings.move and recordings.copy file a recording into +// another container, in its project or another one, the way the web's Move… +// and Copy… dialogs do. Basecamp files in the background, so each action +// starts the filing once through RecordingsService.MoveAndWait or +// CopyAndWait, waits for Basecamp to finish it, and answers with the new +// recording read back at its new id. They replace the model's start-only +// MoveRecording and CopyRecording, which answered with a filing to poll. +// GetFiling moves into the recordings domain beside them, as the check for a +// filing that had not finished when the wait ended. +const ( + moveAction = "move" + copyAction = "copy" +) + +// filingWait bounds how long move and copy wait for Basecamp to finish: the +// SDK's default minute. Tests shorten it. +var filingWait = basecamp.FilingWaitOptions{} + +// recordingRead is one recording as its typed read returns it: the record +// to answer with, and the fields filing needs from it. +type recordingRead struct { + record json.RawMessage + id int64 + typ string + parent *basecamp.Parent +} + +type recordingReader func(ctx context.Context, api API, id int64) (recordingRead, error) + +// filingReads holds the typed read for each recordable type Basecamp moves +// and copies on its own: every type basecamp.FilingContainerFor accepts. A +// to-do list and a to-do list group are both Todolist, and a folder is a +// Vault; FilingContainerFor tells them apart by the parent the read reports. +var filingReads = map[string]recordingReader{ + "Kanban::Card": reader(API.Cards, (*basecamp.CardsService).Get), + "Kanban::Column": reader(API.CardColumns, (*basecamp.CardColumnsService).Get), + "Message": reader(API.Messages, (*basecamp.MessagesService).Get), + "Document": reader(API.Documents, (*basecamp.DocumentsService).Get), + "Upload": reader(API.Uploads, (*basecamp.UploadsService).Get), + "CloudFile": reader(API.CloudFiles, (*basecamp.CloudFilesService).Get), + "GoogleDocument": reader(API.GoogleDocuments, (*basecamp.GoogleDocumentsService).Get), + "Vault": reader(API.Vaults, (*basecamp.VaultsService).Get), + "Schedule::Entry": seriesReader(reader(API.Schedules, (*basecamp.SchedulesService).GetEntry)), + "Question": reader(API.Checkins, (*basecamp.CheckinsService).GetQuestion), + "Inbox::Forward": reader(API.Forwards, (*basecamp.ForwardsService).Get), + "Todolist": reader(API.Todolists, (*basecamp.TodolistsService).Get), + "Todo": reader(API.Todos, (*basecamp.TodosService).Get), +} + +// reader adapts one SDK typed read into a recordingReader. The record goes +// back to the caller as the SDK serializes it, so the answer has the same +// shape as that type's own get action. +func reader[S, T any](service func(API) S, get func(S, context.Context, int64) (*T, error)) recordingReader { + return func(ctx context.Context, api API, id int64) (recordingRead, error) { + record, err := get(service(api), ctx, id) + if err != nil { + return recordingRead{}, err + } + raw, err := json.Marshal(record) + if err != nil { + return recordingRead{}, err + } + var fields struct { + ID int64 `json:"id"` + Type string `json:"type"` + Parent *basecamp.Parent `json:"parent"` + } + if err := json.Unmarshal(raw, &fields); err != nil { + return recordingRead{}, err + } + return recordingRead{record: raw, id: fields.ID, typ: fields.Type, parent: fields.Parent}, nil + } +} + +// occurrenceType is what Basecamp answers a recurring event's read with: it +// redirects to one occurrence, whose parent is the event. +const occurrenceType = "Schedule::Entry::Occurrence" + +// seriesReader reads a recurring event as its series, which is what files: the +// read names the event, with the occurrence it answered with kept as +// occurrence_id. An occurrence's parent is the event, not the schedule, so the +// record carries no parent. +func seriesReader(read recordingReader) recordingReader { + return func(ctx context.Context, api API, id int64) (recordingRead, error) { + r, err := read(ctx, api, id) + if err != nil || r.typ != occurrenceType || r.parent == nil { + return r, err + } + fields := map[string]any{} + decoder := json.NewDecoder(bytes.NewReader(r.record)) + decoder.UseNumber() + if err := decoder.Decode(&fields); err != nil { + return recordingRead{}, err + } + fields["occurrence_id"] = fields["id"] + fields["id"] = r.parent.ID + fields["type"] = r.parent.Type + delete(fields, "parent") + raw, err := json.Marshal(fields) + if err != nil { + return recordingRead{}, err + } + return recordingRead{record: raw, id: r.parent.ID, typ: r.parent.Type}, nil + } +} + +func filingTypes() []any { + types := make([]any, 0, len(filingReads)) + for _, typ := range slices.Sorted(maps.Keys(filingReads)) { + types = append(types, typ) + } + return types +} + +// containerNouns names each kind of container in messages. +var containerNouns = map[basecamp.FilingContainer]string{ + basecamp.FilingIntoCardTable: "card table", + basecamp.FilingIntoMessageBoard: "message board", + basecamp.FilingIntoVault: "Docs & Files", + basecamp.FilingIntoSchedule: "schedule", + basecamp.FilingIntoQuestionnaire: "Automatic Check-ins", + basecamp.FilingIntoInbox: "Email Forwards", + basecamp.FilingIntoTodoset: "To-dos", + basecamp.FilingIntoTodolist: "to-do list", +} + +func filingOperation(kind basecamp.FilingType) *catalog.Operation { + verb, verbs, action, id := "Move", "Moves", moveAction, "MoveRecordingAndWait" + if kind == basecamp.FilingCopy { + verb, verbs, action, id = "Copy", "Copies", copyAction, "CopyRecordingAndWait" + } + lower := strings.ToLower(verb) + properties := map[string]any{ + "recording_id": idSchema("The recording to " + lower + "."), + "recording_type": map[string]any{ + "type": "string", + "enum": filingTypes(), + "description": "The recording's type as Basecamp spells it (its JSON \"type\"). A to-do list group is a Todolist and a folder is a Vault.", + }, + "project_id": idSchema("The project to " + lower + " it into. Its one enabled tool of the recording's kind is the destination: its card table, message board, Docs & Files, schedule, Automatic Check-ins or Email Forwards, or for a to-do or to-do list group its one to-do list. A project with none, or several, is refused with what to pass instead. Pass project_id or container_id, not both."), + "container_id": idSchema("The container to " + lower + " it into, of the recording's kind: a card table, message board, Docs & Files tool, schedule, Automatic Check-ins, Email Forwards or To-dos tool from a project's dock, or a to-do list for a to-do or to-do list group. A folder is not a container; pass its Docs & Files here and the folder as folder_id."), + "folder_id": idSchema("A folder inside the destination Docs & Files to put it in. Only for documents, uploads, cloud files, Google documents and folders."), + } + doc := verbs + " a recording into another container, in its project or another one, as the web's " + verb + "… dialog does. " + + "Composed in basecamp-sdk (RecordingsService." + verb + "AndWait): it reads the recording, resolves the destination, starts the " + lower + " once, waits up to a minute for Basecamp to finish it, and reads back the new recording. Not backed by any single Basecamp endpoint, so this action carries no method and no path.\n\n" + + "Returns the new recording, as its type's own get action returns it, with source_id naming the recording it came from. A " + lower + " always gives the recording a new id. A card lands in the card table's Triage, or Done if it is completed.\n\n" + + "Comments, card steps, check-in answers and other recordings that go with their parent are not filed on their own, and neither is a whole tool (a project's Docs & Files or card table itself).\n\n" + + "A failure says how far it got in error.type, and error.retry_safe says whether calling this action again is safe (a malformed call is refused as plain text instead). recording_unreadable and destination_unreadable are a read before the filing that failed, carrying the read's own error. recording_moved is an id that now redirects to recording_id, because the recording was moved: nothing was filed, which is how a retried move is refused once the first one landed. destination_missing, destination_disabled and destination_ambiguous filed nothing and name what to pass instead (destination_disabled carries the tool_id that basecamp_automation enable_tool turns on; destination_ambiguous lists candidates for container_id). filing_refused filed nothing. filing_unsettled was still running when the wait ended: do not " + lower + " again; check it with get_filing and the filing_id it carries. filing_failed was started and reported failed by Basecamp. filed_unread was filed under recording_id but could not be read back." + if kind == basecamp.FilingCopy { + properties["include_comments"] = map[string]any{ + "type": "boolean", + "description": "Copy the recording's comments too. Defaults to false.", + } + doc += " filing_may_have_started is a start that failed without Basecamp refusing it: a copy may exist, and copying again makes another, so look for it in the destination first." + } else { + doc += " filing_may_have_started is a start that failed without Basecamp refusing it; Basecamp refuses a repeated move, so trying again is safe." + } + return &catalog.Operation{ + ID: id, + Action: action, + Tag: recordingsDomainTag, + Summary: syntheticSummaryTag + lower + " a recording into another container or project, wait for Basecamp to finish, and return the new recording.", + Doc: doc, + BodyRequired: true, + Body: map[string]any{ + "type": "object", + "additionalProperties": false, + "required": []any{"recording_id", "recording_type"}, + "properties": properties, + }, + } +} + +// installFilingOperations replaces the model's start-only MoveRecording and +// CopyRecording with the waiting composites, and moves GetFiling into the +// recordings domain beside them. Each must be where the model put it: a +// sync that renamed or dropped one stops the load instead of serving a +// stale twin or losing the check for an unsettled filing. +func installFilingOperations(cat *catalog.Catalog, recordings *catalog.Domain) error { + for _, id := range []string{"MoveRecording", "CopyRecording"} { + if _, err := takeOperation(cat, id); err != nil { + return err + } + } + getFiling, err := takeOperation(cat, "GetFiling") + if err != nil { + return err + } + getFiling.Summary = "Get the state of a move or copy, by the filing_id a move or copy answered with when it had not finished" + recordings.Operations = append(recordings.Operations, getFiling) + slices.SortFunc(recordings.Operations, func(a, b *catalog.Operation) int { return strings.Compare(a.Action, b.Action) }) + return nil +} + +func takeOperation(cat *catalog.Catalog, id string) (*catalog.Operation, error) { + for _, d := range cat.Domains { + for i, op := range d.Operations { + if op.ID == id { + d.Operations = slices.Delete(d.Operations, i, i+1) + return op, nil + } + } + } + return nil, fmt.Errorf("the model serves no %q operation for the filing actions to take over", id) +} + +// filingRequest is one move or copy as the caller named it. +type filingRequest struct { + kind basecamp.FilingType + recordingID int64 + recordingType string + projectID int64 + containerID int64 + folderID int64 + includeComments bool +} + +func handleFiling(kind basecamp.FilingType) compositeHandler { + return func(ctx context.Context, api API, op *catalog.Operation, params map[string]any) (*mcp.CallToolResult, error) { + req, err := parseFilingRequest(kind, op, params) + if err != nil { + return gateway.ErrorResult("%v", err), nil + } + answer, err := fileRecording(ctx, api, req) + if err != nil { + if failure, ok := errors.AsType[*filingError](err); ok { + return failure.result(), nil + } + return gateway.ErrorResult("%v", err), nil + } + return &mcp.CallToolResult{Content: []mcp.Content{&mcp.TextContent{Text: string(answer)}}}, nil + } +} + +func parseFilingRequest(kind basecamp.FilingType, op *catalog.Operation, params map[string]any) (filingRequest, error) { + req := filingRequest{kind: kind} + if err := refuseUnknownParams(op, params); err != nil { + return req, err + } + var err error + if req.recordingID, err = requiredID(params, "recording_id"); err != nil { + return req, err + } + if req.recordingType, err = optionalString(params, "recording_type"); err != nil { + return req, err + } + if _, ok := filingReads[req.recordingType]; !ok { + return req, fmt.Errorf("parameter %q must be one of the types Basecamp moves and copies on its own (describe the action for the list), got %q", "recording_type", req.recordingType) + } + for name, field := range map[string]*int64{"project_id": &req.projectID, "container_id": &req.containerID, "folder_id": &req.folderID} { + if _, given := params[name]; given { + if *field, err = requiredID(params, name); err != nil { + return req, err + } + } + } + if (req.projectID == 0) == (req.containerID == 0) { + return req, errors.New("pass exactly one of project_id and container_id to name where it goes") + } + if raw, given := params["include_comments"]; given && raw != nil { + include, ok := raw.(bool) + if !ok { + return req, fmt.Errorf("parameter %q must be a boolean, got %T", "include_comments", raw) + } + req.includeComments = include + } + return req, nil +} + +// fileRecording reads the recording, resolves where it goes, files it once +// and waits, then reads back what Basecamp made. +func fileRecording(ctx context.Context, api API, req filingRequest) (json.RawMessage, error) { + src, err := filingReads[req.recordingType](ctx, api, req.recordingID) + if err != nil { + return nil, unreadable("recording_unreadable", fmt.Sprintf("Could not read %s %d", req.recordingType, req.recordingID), err) + } + if src.id != req.recordingID { + // A moved recording's old id redirects to its new one. Filing what + // the redirect found would move a recording again when a lost answer + // to a move is retried, so the caller names it by its new id. + return nil, &filingError{ + Type: "recording_moved", + Message: fmt.Sprintf("Recording %d has been moved and is now %d; nothing was filed. Pass recording_id %d to file it from where it is now", req.recordingID, src.id, src.id), + RetrySafe: true, + RecordingID: src.id, + } + } + if src.typ != req.recordingType { + return nil, fmt.Errorf("recording %d is a %s, not a %s", req.recordingID, src.typ, req.recordingType) + } + var parentType string + if src.parent != nil { + parentType = src.parent.Type + } + container, ok := basecamp.FilingContainerFor(src.typ, parentType) + if !ok { + return nil, &filingError{ + Type: "not_filable", + Message: fmt.Sprintf("%s %d, whose parent is a %s, is not filed on its own: this action files the recordings inside a tool, not a whole tool such as a project's Docs & Files", src.typ, src.id, cmp.Or(parentType, "project")), + RetrySafe: true, + } + } + if req.folderID != 0 && !container.Folders() { + return nil, fmt.Errorf("folder_id applies only to Docs & Files items; a %s goes into a %s", src.typ, containerNouns[container]) + } + + destinationID := req.containerID + if destinationID == 0 { + if destinationID, err = destinationInProject(ctx, api, container, req.projectID); err != nil { + return nil, err + } + } + + recordings := api.Recordings() + var filing *basecamp.Filing + if req.kind == basecamp.FilingCopy { + filing, err = recordings.CopyAndWait(ctx, src.id, &basecamp.CopyRecordingRequest{ + FilingDestinationID: destinationID, + ParentID: req.folderID, + IncludeComments: req.includeComments, + }, &filingWait) + } else { + filing, err = recordings.MoveAndWait(ctx, src.id, &basecamp.MoveRecordingRequest{ + FilingDestinationID: destinationID, + ParentID: req.folderID, + }, &filingWait) + } + if err != nil { + return nil, explainFilingError(req, container, err) + } + + made := filing.DestinationRecording + read, ok := filingReads[made.Type] + if !ok { + read = filingReads[src.typ] + } + filed, err := read(ctx, api, made.ID) + if err != nil { + return nil, &filingError{ + Type: "filed_unread", + Message: fmt.Sprintf("%s %d as %s %d, but could not read it back: %v", pastTense(req.kind), src.id, made.Type, made.ID, err), + RecordingID: made.ID, + AppURL: made.AppURL, + } + } + return withSourceID(filed.record, src.id) +} + +// destinationInProject finds the one container of the kind in a project: its +// enabled dock tool of that name, or for a to-do list, its To-dos tool's one +// list. Only enabled tools count, as in the web's dialogs. +func destinationInProject(ctx context.Context, api API, container basecamp.FilingContainer, projectID int64) (int64, error) { + project, err := api.Projects().Get(ctx, projectID) + if err != nil { + return 0, unreadable("destination_unreadable", fmt.Sprintf("Could not read project %d", projectID), err) + } + toolKind := container + if container == basecamp.FilingIntoTodolist { + toolKind = basecamp.FilingIntoTodoset + } + var enabled []basecamp.DockItem + var disabled *basecamp.DockItem + for _, tool := range project.Dock { + switch { + case tool.Name != string(toolKind): + case tool.Enabled: + enabled = append(enabled, tool) + case disabled == nil: + disabled = &tool + } + } + noun := containerNouns[toolKind] + switch { + case len(enabled) == 0 && disabled != nil: + return 0, &filingError{ + Type: "destination_disabled", + Message: fmt.Sprintf("Project %s's %s (%s) is turned off, so nothing can be moved or copied into it. Turn it on with basecamp_automation enable_tool and toolId %d, or pick another project", project.Name, noun, disabled.Title, disabled.ID), + RetrySafe: true, + ToolID: disabled.ID, + } + case len(enabled) == 0: + return 0, &filingError{ + Type: "destination_missing", + Message: fmt.Sprintf("Project %s has no %s", project.Name, noun), + RetrySafe: true, + } + } + + // The candidates are what container_id takes: the tools themselves, or + // for a to-do list, the lists in every enabled To-dos tool. + candidates := make([]filingCandidate, 0, len(enabled)) + plural := noun + " tools" + if container == basecamp.FilingIntoTodolist { + plural = "to-do lists" + for _, tool := range enabled { + lists, err := api.Todolists().List(ctx, tool.ID, nil) + if err != nil { + return 0, unreadable("destination_unreadable", fmt.Sprintf("Could not list the to-do lists in project %s's %s", project.Name, tool.Title), err) + } + for _, list := range lists.Todolists { + candidates = append(candidates, filingCandidate{ID: list.ID, Title: list.Title}) + } + } + } else { + for _, tool := range enabled { + candidates = append(candidates, filingCandidate{ID: tool.ID, Title: tool.Title}) + } + } + switch len(candidates) { + case 0: + return 0, &filingError{ + Type: "destination_missing", + Message: fmt.Sprintf("Project %s has no %s", project.Name, plural), + RetrySafe: true, + } + case 1: + return candidates[0].ID, nil + } + return 0, &filingError{ + Type: "destination_ambiguous", + Message: fmt.Sprintf("Project %s has %d %s; pass one of them as container_id", project.Name, len(candidates), plural), + RetrySafe: true, + Candidates: candidates, + } +} + +// filingError is a move or copy that did not end with a new recording read +// back: how far it got, and whether calling the action again is safe. +type filingError struct { + Type string `json:"type"` + Message string `json:"message"` + RetrySafe bool `json:"retry_safe"` + FilingID int64 `json:"filing_id,omitempty"` + FilingStatus string `json:"filing_status,omitempty"` + RecordingID int64 `json:"recording_id,omitempty"` + AppURL string `json:"app_url,omitempty"` + ToolID int64 `json:"tool_id,omitempty"` + Candidates []filingCandidate `json:"candidates,omitempty"` +} + +type filingCandidate struct { + ID int64 `json:"id"` + Title string `json:"title"` +} + +func (f *filingError) Error() string { return f.Message } + +// unreadable is a read before the filing that failed: nothing was filed, so +// trying again is safe, and the read's own error says whether it can work. +func unreadable(kind, what string, err error) *filingError { + return &filingError{Type: kind, Message: fmt.Sprintf("%s, so nothing was filed: %v", what, err), RetrySafe: true} +} + +func (f *filingError) result() *mcp.CallToolResult { + result, _ := gateway.JSONResult(map[string]any{"error": f}) + result.IsError = true + return result +} + +// explainFilingError says how far a failed MoveAndWait or CopyAndWait got. +func explainFilingError(req filingRequest, container basecamp.FilingContainer, err error) error { + verb := strings.ToLower(string(req.kind)) + if refused, ok := errors.AsType[*basecamp.FilingRefusedError](err); ok { + message := fmt.Sprintf("Basecamp refused to %s it, and nothing was filed: %v", verb, refused.Err) + if apiErr, ok := errors.AsType[*basecamp.Error](refused.Err); ok && apiErr.Code == basecamp.CodeNotFound { + message += fmt.Sprintf(". Basecamp answers not found when the destination is not a %s this recording can go into, or is the one it is in now; a folder goes in folder_id", containerNouns[container]) + } + return &filingError{Type: "filing_refused", Message: message, RetrySafe: true} + } + if started, ok := errors.AsType[*basecamp.FilingMayHaveStartedError](err); ok { + if req.kind == basecamp.FilingCopy { + return &filingError{ + Type: "filing_may_have_started", + Message: fmt.Sprintf("The request to copy it failed without Basecamp refusing it (%v). Basecamp may have made the copy, and copying again makes another: look for it in the destination first", started.Err), + } + } + return &filingError{ + Type: "filing_may_have_started", + Message: fmt.Sprintf("The request to move it failed without Basecamp refusing it (%v). Basecamp refuses a repeated move, so trying again either moves it or is refused because this one did", started.Err), + RetrySafe: true, + } + } + if failed, ok := errors.AsType[*basecamp.FilingFailedError](err); ok { + return &filingError{ + Type: "filing_failed", + Message: fmt.Sprintf("Basecamp started to %s it and reported filing %d failed. Read the recording before trying again", verb, failed.Filing.ID), + FilingID: failed.Filing.ID, + FilingStatus: string(failed.Filing.Status), + } + } + if unsettled, ok := errors.AsType[*basecamp.FilingUnsettledError](err); ok { + return &filingError{ + Type: "filing_unsettled", + Message: fmt.Sprintf("Basecamp had not finished filing %d when the wait ended (%v). Do not %s it again; check it with get_filing and filingId %d, which names the new recording once it lands", unsettled.Filing.ID, unsettled.Cause, verb, unsettled.Filing.ID), + FilingID: unsettled.Filing.ID, + FilingStatus: string(unsettled.Filing.Status), + } + } + return err +} + +func pastTense(kind basecamp.FilingType) string { + if kind == basecamp.FilingCopy { + return "Copied" + } + return "Moved" +} + +// withSourceID adds source_id, the recording a filing came from, to the +// recording it made. +func withSourceID(record json.RawMessage, sourceID int64) (json.RawMessage, error) { + fields := map[string]any{} + decoder := json.NewDecoder(bytes.NewReader(record)) + decoder.UseNumber() + if err := decoder.Decode(&fields); err != nil { + return nil, err + } + fields["source_id"] = sourceID + return json.Marshal(fields) +} diff --git a/internal/mcpserver/filing_test.go b/internal/mcpserver/filing_test.go new file mode 100644 index 000000000..cd2003b1f --- /dev/null +++ b/internal/mcpserver/filing_test.go @@ -0,0 +1,541 @@ +package mcpserver + +import ( + "encoding/json" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "regexp" + "slices" + "strconv" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/basecamp/mcp/mcptest" +) + +const ( + fakeSourceID = 101 + fakeNewID = 202 + fakeProjectID = 77 + fakeFilingID = 9 + fakeListID = 600 +) + +// fakeBasecamp stands in for Basecamp behind move and copy: one recording of +// a given type and parent, a destination project whose dock the test sets, +// and a filing that runs through the statuses the test lists. +type fakeBasecamp struct { + t *testing.T + recordType string + parentType string + dock []basecamp.DockItem + lists []basecamp.Todolist + // recurring answers each read with an occurrence of the event read, as + // Basecamp does for a recurring event. + recurring bool + // movedTo answers a read of the source with the recording at this id, + // as Basecamp's redirect from a moved recording's old id does. + movedTo int + // unreadable answers reads of these paths with these statuses. + unreadable map[string]int + // listsIn overrides lists for the To-dos tool with that id. + listsIn map[string][]basecamp.Todolist + // start answers the POST that starts the filing; nil starts it. + start http.HandlerFunc + // statuses are what GetFiling reports, one per poll, the last repeated. + statuses []basecamp.FilingStatus + + mu sync.Mutex + started map[string]any + starts int + polls int +} + +var trailingID = regexp.MustCompile(`/(\d+)(\.json)?$`) + +func (f *fakeBasecamp) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + path := r.URL.Path + if status, ok := f.unreadable[strings.TrimSuffix(path, ".json")]; ok { + f.json(w, status, map[string]any{"error": "no"}) + return + } + switch { + case r.Method == http.MethodPost && (strings.HasSuffix(path, "/moves.json") || strings.HasSuffix(path, "/copies.json")): + f.starts++ + body, _ := io.ReadAll(r.Body) + f.started = map[string]any{} + require.NoError(f.t, json.Unmarshal(body, &f.started)) + f.started["path"] = path + if f.start != nil { + f.start(w, r) + return + } + f.json(w, http.StatusCreated, f.filing(basecamp.FilingPending)) + case strings.HasPrefix(path, "/999/filings/"): + status := f.statuses[min(f.polls, len(f.statuses)-1)] + f.polls++ + f.json(w, http.StatusOK, f.filing(status)) + case strings.TrimSuffix(path, ".json") == fmt.Sprintf("/999/projects/%d", fakeProjectID): + f.json(w, http.StatusOK, map[string]any{"id": fakeProjectID, "name": "Dest", "dock": f.dock}) + case strings.HasSuffix(path, "/todolists.json"): + todoset := strings.Split(path, "/")[3] + if lists, ok := f.listsIn[todoset]; ok { + f.json(w, http.StatusOK, lists) + return + } + f.json(w, http.StatusOK, f.lists) + case r.Method == http.MethodGet && trailingID.MatchString(path): + id, _ := strconv.Atoi(trailingID.FindStringSubmatch(path)[1]) + if id != fakeSourceID && id != fakeNewID { + f.t.Errorf("unexpected read: %s", path) + w.WriteHeader(http.StatusNotFound) + return + } + if id == fakeSourceID && f.movedTo != 0 { + id = f.movedTo + } + if f.recurring { + f.json(w, http.StatusOK, map[string]any{ + "id": id + 1, + "type": "Schedule::Entry::Occurrence", + "parent": map[string]any{"id": id, "type": "Schedule::Entry", "title": "Weekly"}, + "bucket": map[string]any{"id": fakeProjectID, "name": "Dest", "type": "Project"}, + }) + return + } + f.json(w, http.StatusOK, map[string]any{ + "id": id, + "type": f.recordType, + "title": "Thing", + "parent": map[string]any{"id": 55, "type": f.parentType, "title": "Old home"}, + "bucket": map[string]any{"id": fakeProjectID, "name": "Dest", "type": "Project"}, + }) + default: + f.t.Errorf("unexpected upstream request: %s %s", r.Method, path) + w.WriteHeader(http.StatusNotFound) + } +} + +func (f *fakeBasecamp) filing(status basecamp.FilingStatus) map[string]any { + filing := map[string]any{"id": fakeFilingID, "status": status, "type": "Move", "source_recording_id": fakeSourceID, "url": "x"} + if status == basecamp.FilingCompleted { + filing["destination_recording"] = map[string]any{"id": fakeNewID, "type": f.recordType, "url": "x", "app_url": "https://3.basecamp.com/999/x/202"} + } + return filing +} + +func (f *fakeBasecamp) json(w http.ResponseWriter, status int, body any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + require.NoError(f.t, json.NewEncoder(w).Encode(body)) +} + +// dockFor is a destination project's dock with one enabled tool of every +// kind, each with its own id. +func dockFor() []basecamp.DockItem { + names := []basecamp.FilingContainer{ + basecamp.FilingIntoCardTable, basecamp.FilingIntoMessageBoard, basecamp.FilingIntoVault, basecamp.FilingIntoSchedule, + basecamp.FilingIntoQuestionnaire, basecamp.FilingIntoInbox, basecamp.FilingIntoTodoset, + } + dock := make([]basecamp.DockItem, len(names)) + for i, name := range names { + dock[i] = basecamp.DockItem{ID: int64(500 + i), Name: string(name), Title: string(name), Enabled: true} + } + return dock +} + +func newFake(t *testing.T, recordType, parentType string) *fakeBasecamp { + return &fakeBasecamp{ + t: t, + recordType: recordType, + parentType: parentType, + dock: dockFor(), + lists: []basecamp.Todolist{{ID: fakeListID, Title: "Only list"}}, + statuses: []basecamp.FilingStatus{basecamp.FilingProcessing, basecamp.FilingCompleted}, + } +} + +func filingSession(t *testing.T, fake *fakeBasecamp, cfg Config) *mcp.ClientSession { + t.Helper() + upstream := httptest.NewServer(fake) + t.Cleanup(upstream.Close) + srv, err := New(newTestAPI(upstream), cfg) + require.NoError(t, err) + return mcptest.Connect(t, srv.BuildMCPServer(slog.New(slog.DiscardHandler))) +} + +func file(t *testing.T, session *mcp.ClientSession, action string, params map[string]any) (map[string]any, bool) { + t.Helper() + text, isError := mcptest.CallText(t, session, "basecamp_recordings", map[string]any{"action": action, "params": params}) + var decoded map[string]any + if err := json.Unmarshal([]byte(text), &decoded); err != nil { + return map[string]any{"text": text}, isError + } + return decoded, isError +} + +func failureOf(t *testing.T, answer map[string]any) map[string]any { + t.Helper() + failure, ok := answer["error"].(map[string]any) + require.True(t, ok, "not a filing failure: %v", answer) + return failure +} + +func dockID(name basecamp.FilingContainer) float64 { + for _, tool := range dockFor() { + if tool.Name == string(name) { + return float64(tool.ID) + } + } + return 0 +} + +// recordableTypes is every recording type a candidate for filing could be, +// as Basecamp spells it in JSON: bc3's recordable models (app/models, each +// including Recordable), the SDK's RecordingType constants, and every +// type-shaped string in the vendored model. FilingContainerFor is asked +// about each of them, with each as a parent, so a type it starts accepting +// shows up here without anyone listing it. +func recordableTypes(t *testing.T) []string { + t.Helper() + seen := map[string]bool{} + for _, typ := range []string{ + "Attachment", "AutolinkedImage", "Chat::Lines::Code", "Chat::Lines::RichText", "Chat::Lines::Text", + "Chat::Lines::Upload", "Chat::Transcript", "Client::Approval", "Client::Approval::Response", "Client::Board", + "Client::Correspondence", "Client::Forward", "Client::Introduction", "Client::Reply", "CloudFile", "Comment", + "Dock", "Document", "Door", "Embed", "Gallery", "Gauge", "Gauge::Needle", "GoogleDocument", "Hill::Dot", + "Hill::Version", "Inbox", "Inbox::Forward", "Inbox::Reply", "Journal", "Journal::Entry", "Kanban::Board", + "Kanban::Boardset", "Kanban::Card", "Kanban::Column", "Kanban::DoneColumn", "Kanban::NotNowColumn", + "Kanban::OnHold", "Kanban::Step", "Kanban::Triage", "Kanban::Wormhole", "Message", "Message::Board", + "Notebook::Note", "Printout", "Question", "Question::Answer", "Questionnaire", "Relay", "Schedule", + "Schedule::Entry", "Schedule::Entry::Occurrence", "Subtask", "Timeline", "Timesheet", "Timesheet::Entry", + "Todo", "Todolist", "Todoset", "Upload", "Vault", + string(basecamp.RecordingTypeComment), string(basecamp.RecordingTypeDocument), string(basecamp.RecordingTypeDoor), + string(basecamp.RecordingTypeKanbanCard), string(basecamp.RecordingTypeKanbanStep), string(basecamp.RecordingTypeMessage), + string(basecamp.RecordingTypeQuestionAnswer), string(basecamp.RecordingTypeScheduleEntry), string(basecamp.RecordingTypeTodo), + string(basecamp.RecordingTypeTodolist), string(basecamp.RecordingTypeUpload), string(basecamp.RecordingTypeVault), + } { + seen[typ] = true + } + model, err := os.ReadFile("model/openapi.json") + require.NoError(t, err) + for _, match := range regexp.MustCompile(`"([A-Z][A-Za-z]+(?:::[A-Z][A-Za-z]+)*)"`).FindAllSubmatch(model, -1) { + seen[string(match[1])] = true + } + types := make([]string, 0, len(seen)) + for typ := range seen { + types = append(types, typ) + } + slices.Sort(types) + return types +} + +// TestFilingServesEveryTypeTheSDKFiles is the parity check: every recording +// type basecamp.FilingContainerFor accepts, under every parent it accepts it +// with, moves and copies through basecamp_recordings into a project's +// container of the kind the SDK names, and answers with the new recording. +// A type the SDK starts filing that this server cannot read, or a container +// kind it cannot resolve from a project, fails here. +func TestFilingServesEveryTypeTheSDKFiles(t *testing.T) { + type route struct { + typ string + container basecamp.FilingContainer + } + types := recordableTypes(t) + routes := map[route]string{} + for _, typ := range types { + for _, parent := range append([]string{""}, types...) { + if container, ok := basecamp.FilingContainerFor(typ, parent); ok { + if _, seen := routes[route{typ, container}]; !seen { + routes[route{typ, container}] = parent + } + } + } + } + require.NotEmpty(t, routes) + t.Logf("%d routes from %d candidate types", len(routes), len(types)) + + served := map[string]bool{} + for r, parent := range routes { + served[r.typ] = true + for _, action := range []string{moveAction, copyAction} { + t.Run(fmt.Sprintf("%s %s under %s", action, r.typ, parent), func(t *testing.T) { + t.Parallel() + fake := newFake(t, r.typ, parent) + fake.statuses = []basecamp.FilingStatus{basecamp.FilingCompleted} + session := filingSession(t, fake, Config{}) + + answer, isError := file(t, session, action, map[string]any{ + "recording_id": fakeSourceID, + "recording_type": r.typ, + "project_id": fakeProjectID, + }) + require.False(t, isError, "%v", answer) + assert.Equal(t, float64(fakeNewID), answer["id"]) + assert.Equal(t, float64(fakeSourceID), answer["source_id"]) + assert.Equal(t, r.typ, answer["type"]) + + want := dockID(r.container) + if r.container == basecamp.FilingIntoTodolist { + want = fakeListID + } + assert.Equal(t, want, fake.started["filing_destination_id"], "filed into the project's %s", r.container) + assert.Equal(t, 1, fake.starts, "started once") + }) + } + } + for typ := range filingReads { + assert.True(t, served[typ], "recording_type advertises %s, which FilingContainerFor never accepts", typ) + } +} + +func TestFilingIntoANamedContainerSkipsTheProject(t *testing.T) { + fake := newFake(t, "Document", "Vault") + session := filingSession(t, fake, Config{}) + + answer, isError := file(t, session, copyAction, map[string]any{ + "recording_id": fakeSourceID, + "recording_type": "Document", + "container_id": "4100", + "folder_id": 4200, + "include_comments": true, + }) + require.False(t, isError, "%v", answer) + assert.Equal(t, float64(4100), fake.started["filing_destination_id"]) + assert.Equal(t, float64(4200), fake.started["parent_id"]) + assert.Equal(t, true, fake.started["include_comments"]) + assert.Contains(t, fake.started["path"], "/copies.json") +} + +func TestFilingRefusesADestinationItCannotPick(t *testing.T) { + t.Run("turned off", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.dock = []basecamp.DockItem{{ID: 900, Name: "message_board", Title: "Message Board", Enabled: false}} + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), moveAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Message", "project_id": fakeProjectID, + })) + assert.Equal(t, "destination_disabled", failure["type"]) + assert.Equal(t, float64(900), failure["tool_id"]) + assert.Equal(t, true, failure["retry_safe"]) + assert.Contains(t, failure["message"], "enable_tool") + assert.Zero(t, fake.starts) + }) + t.Run("several", func(t *testing.T) { + fake := newFake(t, "Kanban::Card", "Kanban::Triage") + fake.dock = []basecamp.DockItem{ + {ID: 901, Name: "kanban_board", Title: "Bugs", Enabled: true}, + {ID: 902, Name: "kanban_board", Title: "Ideas", Enabled: true}, + } + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), copyAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Kanban::Card", "project_id": fakeProjectID, + })) + assert.Equal(t, "destination_ambiguous", failure["type"]) + assert.Equal(t, []any{ + map[string]any{"id": float64(901), "title": "Bugs"}, + map[string]any{"id": float64(902), "title": "Ideas"}, + }, failure["candidates"]) + assert.Zero(t, fake.starts) + }) + t.Run("several to-do lists", func(t *testing.T) { + fake := newFake(t, "Todo", "Todolist") + fake.lists = []basecamp.Todolist{{ID: 601, Title: "A"}, {ID: 602, Title: "B"}} + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), moveAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Todo", "project_id": fakeProjectID, + })) + assert.Equal(t, "destination_ambiguous", failure["type"]) + assert.Len(t, failure["candidates"], 2) + assert.Zero(t, fake.starts) + }) + t.Run("to-do lists across several To-dos tools", func(t *testing.T) { + fake := newFake(t, "Todo", "Todolist") + fake.dock = append(fake.dock, basecamp.DockItem{ID: 599, Name: "todoset", Title: "Second", Enabled: true}) + fake.listsIn = map[string][]basecamp.Todolist{"599": {{ID: 601, Title: "Second list"}}} + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), copyAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Todo", "project_id": fakeProjectID, + })) + assert.Equal(t, "destination_ambiguous", failure["type"]) + assert.Equal(t, []any{ + map[string]any{"id": float64(fakeListID), "title": "Only list"}, + map[string]any{"id": float64(601), "title": "Second list"}, + }, failure["candidates"], "the candidates are lists, which container_id takes, not To-dos tools") + assert.Zero(t, fake.starts) + }) + t.Run("none", func(t *testing.T) { + fake := newFake(t, "Inbox::Forward", "Inbox") + fake.dock = nil + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), moveAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Inbox::Forward", "project_id": fakeProjectID, + })) + assert.Equal(t, "destination_missing", failure["type"]) + assert.Zero(t, fake.starts) + }) +} + +func mustFail(t *testing.T, session *mcp.ClientSession, action string, params map[string]any) map[string]any { + t.Helper() + answer, isError := file(t, session, action, params) + require.True(t, isError, "%v", answer) + return answer +} + +func TestFilingRefusesWhatItCannotFile(t *testing.T) { + cases := map[string]struct { + parentType string + params map[string]any + want string + }{ + "a whole Docs & Files tool": {"Project", map[string]any{"recording_type": "Vault", "project_id": fakeProjectID}, "not filed on its own"}, + "a folder outside Docs & Files": {"Message::Board", map[string]any{"recording_type": "Message", "container_id": 5, "folder_id": 6}, "folder_id applies only"}, + "both destinations": {"Message::Board", map[string]any{"recording_type": "Message", "container_id": 5, "project_id": fakeProjectID}, "exactly one"}, + "no destination": {"Message::Board", map[string]any{"recording_type": "Message"}, "exactly one"}, + "a type that is not filed": {"Message", map[string]any{"recording_type": "Comment", "container_id": 5}, "recording_type"}, + "comments on a move": {"Message::Board", map[string]any{"recording_type": "Message", "container_id": 5, "include_comments": true}, "unknown parameter"}, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + typ, _ := c.params["recording_type"].(string) + fake := newFake(t, typ, c.parentType) + params := map[string]any{"recording_id": fakeSourceID} + for k, v := range c.params { + params[k] = v + } + answer := mustFail(t, filingSession(t, fake, Config{}), moveAction, params) + assert.Contains(t, fmt.Sprint(answer), c.want) + assert.Zero(t, fake.starts, "nothing filed") + }) + } +} + +// TestFilingSaysWhetherTryingAgainIsSafe pins how each way a filing can end +// short of a new recording reads to the caller. +func TestFilingSaysWhetherTryingAgainIsSafe(t *testing.T) { + defer func(saved basecamp.FilingWaitOptions) { filingWait = saved }(filingWait) + filingWait.Timeout = 300 * time.Millisecond + + params := map[string]any{"recording_id": fakeSourceID, "recording_type": "Message", "container_id": 5} + + t.Run("refused", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.start = func(w http.ResponseWriter, _ *http.Request) { + fake.json(w, http.StatusUnprocessableEntity, map[string]any{"error": "nope"}) + } + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), copyAction, params)) + assert.Equal(t, "filing_refused", failure["type"]) + assert.Equal(t, true, failure["retry_safe"]) + }) + t.Run("refused as not found", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.start = func(w http.ResponseWriter, _ *http.Request) { + fake.json(w, http.StatusNotFound, map[string]any{"error": "not found"}) + } + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), moveAction, params)) + assert.Equal(t, "filing_refused", failure["type"]) + assert.Contains(t, failure["message"], "not a message board this recording can go into, or is the one it is in now") + }) + t.Run("copy may have started", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.start = func(w http.ResponseWriter, _ *http.Request) { + fake.json(w, http.StatusInternalServerError, map[string]any{"error": "boom"}) + } + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), copyAction, params)) + assert.Equal(t, "filing_may_have_started", failure["type"]) + assert.Equal(t, false, failure["retry_safe"]) + assert.Equal(t, 1, fake.starts, "a copy is never sent twice") + }) + t.Run("failed", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.statuses = []basecamp.FilingStatus{basecamp.FilingFailed} + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), moveAction, params)) + assert.Equal(t, "filing_failed", failure["type"]) + assert.Equal(t, float64(fakeFilingID), failure["filing_id"]) + assert.Equal(t, false, failure["retry_safe"]) + }) + t.Run("unsettled", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.statuses = []basecamp.FilingStatus{basecamp.FilingProcessing} + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), copyAction, params)) + assert.Equal(t, "filing_unsettled", failure["type"]) + assert.Equal(t, float64(fakeFilingID), failure["filing_id"]) + assert.Equal(t, "processing", failure["filing_status"]) + assert.Equal(t, false, failure["retry_safe"]) + assert.Contains(t, failure["message"], "get_filing") + }) +} + +func TestFilingIsAWriteTheReadOnlyServerDrops(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + session := filingSession(t, fake, Config{ReadOnly: true}) + + answer := mustFail(t, session, moveAction, map[string]any{"recording_id": fakeSourceID, "recording_type": "Message", "container_id": 5}) + assert.NotContains(t, fmt.Sprint(answer), "source_id") + assert.Zero(t, fake.starts) + + text, isError := mcptest.CallText(t, session, "basecamp_recordings", map[string]any{"action": "describe", "params": map[string]any{"action": "get_filing"}}) + assert.False(t, isError, "get_filing stays served read-only: %s", text) +} + +func TestFilingFilesARecurringEventAsItsSeries(t *testing.T) { + fake := newFake(t, "Schedule::Entry", "Schedule") + fake.recurring = true + session := filingSession(t, fake, Config{}) + + answer, isError := file(t, session, moveAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Schedule::Entry", "project_id": fakeProjectID, + }) + require.False(t, isError, "%v", answer) + assert.Equal(t, fmt.Sprintf("/999/recordings/%d/moves.json", fakeSourceID), fake.started["path"], "the series is what moves") + assert.Equal(t, float64(fakeNewID), answer["id"]) + assert.Equal(t, float64(fakeNewID+1), answer["occurrence_id"]) + assert.Equal(t, "Schedule::Entry", answer["type"]) + assert.Equal(t, float64(fakeSourceID), answer["source_id"]) + assert.NotContains(t, answer, "parent") +} + +func TestFilingTypesAFailedReadBeforeFiling(t *testing.T) { + t.Run("the recording", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.unreadable = map[string]int{fmt.Sprintf("/999/messages/%d", fakeSourceID): http.StatusNotFound} + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), moveAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Message", "project_id": fakeProjectID, + })) + assert.Equal(t, "recording_unreadable", failure["type"]) + assert.Equal(t, true, failure["retry_safe"]) + assert.Zero(t, fake.starts) + }) + t.Run("the destination project", func(t *testing.T) { + fake := newFake(t, "Message", "Message::Board") + fake.unreadable = map[string]int{fmt.Sprintf("/999/projects/%d", fakeProjectID): http.StatusForbidden} + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), copyAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Message", "project_id": fakeProjectID, + })) + assert.Equal(t, "destination_unreadable", failure["type"]) + assert.Equal(t, true, failure["retry_safe"]) + assert.Zero(t, fake.starts) + }) +} + +func TestFilingRefusesAnIDThatNowRedirects(t *testing.T) { + fake := newFake(t, "Document", "Vault") + fake.movedTo = fakeNewID + failure := failureOf(t, mustFail(t, filingSession(t, fake, Config{}), moveAction, map[string]any{ + "recording_id": fakeSourceID, "recording_type": "Document", "container_id": 5, + })) + assert.Equal(t, "recording_moved", failure["type"]) + assert.Equal(t, float64(fakeNewID), failure["recording_id"]) + assert.Equal(t, true, failure["retry_safe"]) + assert.Zero(t, fake.starts, "a retried move never files the recording again") +} diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index f1e5d3018..b58d4348f 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -1,7 +1,7 @@ { "source": "github.com/basecamp/basecamp-sdk", - "commit": "b2e9356f386d7ca5fc87d0ba89ad197ffff9af5c", - "ref": "v0.31.0", + "commit": "13227457da654997d78d3549ab7a0191825fda97", + "ref": "go/v0.32.0", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", "patches": "binary-upload operations dropped because the toolkit refuses their non-JSON bodies (EXCLUDED_OPERATIONS) and the stream-ticket mint dropped by policy (POLICY_EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" diff --git a/internal/mcpserver/testdata/catalog_snapshot.txt b/internal/mcpserver/testdata/catalog_snapshot.txt index ade56d087..76c7ba4ed 100644 --- a/internal/mcpserver/testdata/catalog_snapshot.txt +++ b/internal/mcpserver/testdata/catalog_snapshot.txt @@ -241,7 +241,6 @@ Call {"action": "describe", "params": {"action": "NAME"}} for an action's full p ACTIONS (RO = read-only): - archive_recording: Archive a recording; bc3 answers 403 for types it never lets be archived, timesheet entries among them -- copy_recording: Start copying a recording into another project; poll GetFiling for the result - create_answer: Create a new answer for a question - create_lineup_marker: Create a new lineup marker - create_project_from_template: Create a project from a template (asynchronous) @@ -261,7 +260,6 @@ ACTIONS (RO = read-only): - enable_tool: Enable a tool (show it on the project dock) - get_answer (RO): Get a single answer by id - get_answers_by_person (RO, paginated): Get all answers from a specific person for a question -- get_filing (RO): Get the state of a move or copy started by MoveRecording or CopyRecording - get_project_construction (RO): Get the status of a project construction - get_question (RO): Get a single question by id - get_question_reminders (RO, paginated): Get pending check-in reminders for the current user @@ -282,7 +280,6 @@ ACTIONS (RO = read-only): - list_recordings (RO, paginated): List recordings of a given type across projects - list_templates (RO, paginated): List all templates visible to the current user - list_webhooks (RO, paginated): List all webhooks for a project -- move_recording: Start moving a recording into another project; poll GetFiling for the result - move_recording_to_vault: Move a document, upload or vault into another vault in its project, or reorder it in its own vault - pause_question: Pause a check-in question (stops sending reminders) - reposition_tool: Reposition a tool on the project dock @@ -413,11 +410,14 @@ ACTIONS (RO = read-only): - update_my_note: Replace the note's content, recording a new revision server-side == basecamp_recordings -Recordings: the compact projection of one recording, resolved from the pointer an account event feed row carries. +Recordings: the compact projection of one recording, resolved from the pointer an account event feed row carries; and moving or copying a recording into another container or project, waiting for Basecamp to finish and returning the new recording. Gateway tool: call with {"action": "...", "params": {...}}. Call {"action": "describe", "params": {"action": "NAME"}} for an action's full parameter schema. ACTIONS (RO = read-only): +- copy: Synthetic: copy a recording into another container or project, wait for Basecamp to finish, and return the new recording. +- get_filing (RO): Get the state of a move or copy, by the filing_id a move or copy answered with when it had not finished +- move: Synthetic: move a recording into another container or project, wait for Basecamp to finish, and return the new recording. - summarize (RO): Synthetic: resolve a recording pointer into a compact projection. diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index bb75db60d..e81547c3a 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.31.0", - "revision": "b2e9356f386d", - "updated_at": "2026-10-28T04:21:17Z" + "version": "v0.32.0", + "revision": "13227457da65", + "updated_at": "2026-10-28T19:25:31Z" }, "api": { "repo": "basecamp/bc3", diff --git a/nix/package.nix b/nix/package.nix index ad5912ef5..4ed9c71bc 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -8,7 +8,7 @@ buildGoModule.override { go = go_1_26; } (finalAttrs: { src = lib.cleanSource ./..; # To update: set to lib.fakeHash, run `nix build`, use the hash from the error. - vendorHash = "sha256-K/+fC4LChVQRYXOG6xIt/2j4KPnh0/d7nuqAuekTzGw="; + vendorHash = "sha256-HFrhx9UJa/emaHZf4xcoQVglrTUepJRZDRofkZI5+No="; subPackages = [ "cmd/basecamp" ];