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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ Common variables (subset; see command READMEs for complete lists):
- The MCP SDK validates a tool's `InputSchema` before the handler runs, so a parameter listed in `Required` can never be defaulted or aliased in Go. Widening a shipped tool's parameter while still accepting the old form (as `move_task_to_workflow_stage` does for the scalar `task_id` it advertised before it took `task_ids`) means leaving the new parameter out of `Required` and enforcing it in the handler.
- Annotation hints are mandatory and must be explicit: every tool sets `ReadOnlyHint`, `DestructiveHint` and `OpenWorldHint`. The latter two are `*bool` in the SDK, so a nil value is omitted from `tools/list` and the spec then defaults it to `true`; OpenAI's app review rejects tools with missing hints. Use `new(false)` unless the tool truly destroys data (deletes, plus Desk ticket create/reply, which email the customer) or reaches outside the customer's Teamwork account (help-doc articles published to a public knowledge base, Desk ticket create/reply). `TestAnnotationHintsAreExplicit` in `cmd/docs-gen/main_test.go` guards this across all four products with `allowDelete=true`.
- JSON-Schema gotcha (OpenAI Responses API): every `Type: "array"` node — including inside `AnyOf`/`OneOf`/`AllOf` branches — must declare `Items`. OpenAI rejects bare arrays at tool-registration time even with `strict: false`; Anthropic does not, so Claude Desktop hides the bug. `TestToolInputSchemasArrayItems` in `internal/twprojects/tools_test.go` guards this — if it fires, pick the right item schema rather than weakening the test.
- Any output schema built by reflection over an SDK response must be generated through `helpers.WithDateTypeSchema(...)`. `twapi.Date` and `twapi.OptionalDateTime` are defined over `time.Time`, so `jsonschema.For` describes them by `time.Time`'s unexported fields — an opaque `object` — while their `MarshalJSON` writes a string. Every response carrying one then fails output-schema validation at any validating client, on every call, for every row; non-validating clients see nothing wrong, so this ships silently. A response picks the types up transitively (`projects.Team` is the only model with an `OptionalDateTime`, but `SearchResponse` sideloads `Team`), so register the override rather than reasoning about which models are affected — pre-existing `TypeSchemas` entries are preserved, as `teams.go` does for `LegacyNumber`. Two traps when testing: the mocks in `*_test.go` usually reply `{}`, which leaves such fields nil and encodes as `null`, exercising neither the set nor the unset case; and neither the server nor `testutil.ExecuteToolRequest` validates structured content, so a mismatch is invisible in-process. `TestTeamDeletedDateValidatesAgainstOutputSchema` in `internal/twprojects/teams_test.go` validates the result against the tool's published schema by hand — copy that shape rather than assuming a green tool test means the wire shape is right.
- Date and date-time parameters go through the binders in `internal/helpers/tool_parser.go`, which accept more than one layout (see `dateTimeLayouts` in `internal/helpers/datetime.go`): RFC 3339, an offset-less date-time, and a plain `YYYY-MM-DD`. Models emit the plain date by default when asked about a range, so a strict RFC 3339 parse costs a failed first call and a visible retry. Do not narrow this back. Use `helpers.DateTimeFilterSchema(...)` rather than an inline schema so every filter advertises both forms, and pass `helpers.EndOfDay()` to the binder for any *upper-bound* filter (`end_date`, `*_before`) — a date-only value there must resolve to the day's last second, or the range silently drops its closing day. Handlers that forward the value as a raw query-string parameter instead of binding it use `helpers.NormalizeDateTime(...)`.
- The calendar events endpoint binds `fields[calendarsEvents]`, not `fields[events]` (SDK v1.20.8+ sends the right key via a `sparsefields:key` marker), and its server-side filtering was broken until an API fix in 2026-08. Verify against the live API before assuming an endpoint honours `fields[...]`.
- `list_*` tools follow a specific contract — see `TaskList` in `internal/twprojects/tasks.go` as the canonical pattern:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/sonh/qs v0.7.0
github.com/teamwork/desksdkgo v1.1.0
github.com/teamwork/spacessdkgo v0.0.0-20260518181558-a6af69d00abb
github.com/teamwork/twapi-go-sdk v1.21.3
github.com/teamwork/twapi-go-sdk v1.21.4
)

require (
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ github.com/teamwork/twapi-go-sdk v1.21.2 h1:SsLaxc15Q+m1v+LO0UHWoxCJ+I4wf6uTftfV
github.com/teamwork/twapi-go-sdk v1.21.2/go.mod h1:uj6BNtyyKtggfeY3whzn8bLWuhP1twhghjWD6z3h3F4=
github.com/teamwork/twapi-go-sdk v1.21.3 h1:xTE2l2Xfw9WQLIlWYUNazQM4y4N6K3Y34CJuUO0xTSM=
github.com/teamwork/twapi-go-sdk v1.21.3/go.mod h1:uj6BNtyyKtggfeY3whzn8bLWuhP1twhghjWD6z3h3F4=
github.com/teamwork/twapi-go-sdk v1.21.4 h1:ELySgyUMeSuyrZzuhU0+4bjBgf8X8pZEMAqVxm+7J5o=
github.com/teamwork/twapi-go-sdk v1.21.4/go.mod h1:uj6BNtyyKtggfeY3whzn8bLWuhP1twhghjWD6z3h3F4=
github.com/tinylib/msgp v1.6.3 h1:bCSxiTz386UTgyT1i0MSCvdbWjVW+8sG3PjkGsZQt4s=
github.com/tinylib/msgp v1.6.3/go.mod h1:RSp0LW9oSxFut3KzESt5Voq4GVWyS+PSulT77roAqEA=
github.com/tklauser/go-sysconf v0.3.16 h1:frioLaCQSsF5Cy1jgRBrzr6t502KIIwQ0MArYICU0nA=
Expand Down
25 changes: 19 additions & 6 deletions internal/helpers/schema_date.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,22 @@ import (
twapi "github.com/teamwork/twapi-go-sdk"
)

// WithDateTypeSchema registers a JSON-schema override for the twapi.Date type
// on the given generation options. twapi.Date is defined as `type Date
// time.Time`, so the reflection-based generator would otherwise emit a useless
// object schema for time.Time's unexported fields. This override forces it to a
// nullable, date-only string.
// WithDateTypeSchema registers JSON-schema overrides for the SDK's date types
// on the given generation options. Both twapi.Date and twapi.OptionalDateTime
// are defined over time.Time (`type Date time.Time`), so the reflection-based
// generator would otherwise emit a useless object schema for time.Time's
// unexported fields — while their MarshalJSON methods emit a string. Every
// response carrying one then fails output-schema validation on every call. The
// overrides force them to nullable strings matching what the marshaller
// actually writes.
//
// The types are covered here rather than at each call site because a response
// picks them up transitively: projects.Team is the only model with an
// OptionalDateTime field, but SearchResponse sideloads Team, so the search
// schema needs the same override.
//
// Use it whenever generating an output schema from a response type that carries
// (or sideloads) twapi.Date fields:
// (or sideloads) those fields:
//
// schema, err = jsonschema.For[Response](helpers.WithDateTypeSchema(&jsonschema.ForOptions{}))
//
Expand All @@ -33,5 +41,10 @@ func WithDateTypeSchema(opts *jsonschema.ForOptions) *jsonschema.ForOptions {
Format: "date",
Description: "Null or date-only date string",
}
opts.TypeSchemas[reflect.TypeFor[twapi.OptionalDateTime]()] = &jsonschema.Schema{
Types: []string{"null", "string"},
Format: "date-time",
Description: "Null or RFC3339 date-time string. Null when the value is unset.",
}
return opts
}
8 changes: 4 additions & 4 deletions internal/twprojects/teams.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,26 @@ func init() {
var err error

// generate the output schemas only once
teamGetOutputSchema, err = jsonschema.For[projects.TeamGetResponse](&jsonschema.ForOptions{
teamGetOutputSchema, err = jsonschema.For[projects.TeamGetResponse](helpers.WithDateTypeSchema(&jsonschema.ForOptions{
TypeSchemas: map[reflect.Type]*jsonschema.Schema{
reflect.TypeFor[projects.LegacyNumber](): {
Type: "string",
Description: "A numeric value that is returned as a string.",
},
},
})
}))
if err != nil {
panic(fmt.Sprintf("failed to generate JSON schema for TeamGetResponse: %v", err))
}
helpers.WithMetaWebLinkSchema(teamGetOutputSchema)
teamListOutputSchema, err = jsonschema.For[projects.TeamListResponse](&jsonschema.ForOptions{
teamListOutputSchema, err = jsonschema.For[projects.TeamListResponse](helpers.WithDateTypeSchema(&jsonschema.ForOptions{
TypeSchemas: map[reflect.Type]*jsonschema.Schema{
reflect.TypeFor[projects.LegacyNumber](): {
Type: "string",
Description: "A numeric value that is returned as a string.",
},
},
})
}))
if err != nil {
panic(fmt.Sprintf("failed to generate JSON schema for TeamListResponse: %v", err))
}
Expand Down
114 changes: 114 additions & 0 deletions internal/twprojects/teams_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package twprojects_test

import (
"encoding/json"
"net/http"
"testing"

"github.com/google/jsonschema-go/jsonschema"
"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/teamwork/mcp/internal/testutil"
"github.com/teamwork/mcp/internal/twprojects"
)
Expand Down Expand Up @@ -66,6 +69,117 @@ func TestTeamList(t *testing.T) {
})
}

// TestTeamDeletedDateValidatesAgainstOutputSchema pins the JSON shape of the
// team `deletedDate` against the tools' published output schemas. It is the
// SDK's only twapi.OptionalDateTime field, and that type is defined over
// time.Time, so the reflected schema described it as an object while
// MarshalJSON emitted an RFC3339 string — every validating client discarded the
// whole response.
//
// The other team tests pass `{}` as the body, which leaves DeletedAt nil and
// encodes as null, so they never exercised either half. Both cases belong here:
// the empty string the API sends for every live team (encoding/json allocates
// the pointer before UnmarshalJSON runs, so it survives as a non-nil pointer to
// the zero time), and the timestamp it sends for a deleted one.
func TestTeamDeletedDateValidatesAgainstOutputSchema(t *testing.T) {
tests := []struct {
name string
deletedDate string
}{
{name: "live team", deletedDate: `""`},
{name: "deleted team", deletedDate: `"2026-01-02T03:04:05Z"`},
{name: "null", deletedDate: `null`},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
team := `{"id":"123","name":"Example","deletedDate":` + tt.deletedDate + `}`

mcpServer := mcpServerMock(t, http.StatusOK, []byte(`{"team":`+team+`}`))
testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodTeamGet.String(), map[string]any{
"id": float64(123),
}, testutil.ExecuteToolRequestWithCheckMessage(
checkStructuredContentMatchesOutputSchema(twprojects.MethodTeamGet.String()),
))

mcpServer = mcpServerMock(t, http.StatusOK, []byte(`{"teams":[`+team+`]}`))
testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodTeamList.String(), map[string]any{
"page": float64(1),
"page_size": float64(10),
}, testutil.ExecuteToolRequestWithCheckMessage(
checkStructuredContentMatchesOutputSchema(twprojects.MethodTeamList.String()),
))
})
}
}

// checkStructuredContentMatchesOutputSchema returns a check that validates a
// tool result's StructuredContent against the output schema the named tool
// publishes in tools/list.
//
// Neither the test harness nor the server validates this, so a mismatch is
// invisible in-process and only surfaces at a validating client, which then
// discards a response the server returned successfully. StructuredContent holds
// the Go value rather than decoded JSON, so it has to be round-tripped through
// encoding/json to see what the client will actually receive.
func checkStructuredContentMatchesOutputSchema(toolName string) func(t *testing.T, result mcp.Result) {
return func(t *testing.T, result mcp.Result) {
t.Helper()

testutil.CheckMessage(t, result)

toolResult, ok := result.(*mcp.CallToolResult)
if !ok {
t.Fatalf("unexpected result type: %T", result)
}
if toolResult.StructuredContent == nil {
t.Fatalf("tool %s returned no structured content", toolName)
}

schema := outputSchemaFor(t, toolName)
resolved, err := schema.Resolve(nil)
if err != nil {
t.Fatalf("failed to resolve output schema for %s: %s", toolName, err)
}

encoded, err := json.Marshal(toolResult.StructuredContent)
if err != nil {
t.Fatalf("failed to encode structured content for %s: %s", toolName, err)
}
var decoded any
if err := json.Unmarshal(encoded, &decoded); err != nil {
t.Fatalf("failed to decode structured content for %s: %s", toolName, err)
}

if err := resolved.Validate(decoded); err != nil {
t.Errorf("tool %s: structured content does not match its output schema: %s\nbody: %s",
toolName, err, encoded)
}
}
}

// outputSchemaFor looks up the output schema a registered tool publishes.
func outputSchemaFor(t *testing.T, toolName string) *jsonschema.Schema {
t.Helper()

group := twprojects.DefaultToolsetGroup(false, true, testutil.ProjectsEngineMock(http.StatusOK, nil))
for _, toolset := range group.Toolsets {
for _, tool := range toolset.GetAvailableTools() {
if tool.Tool.Name != toolName {
continue
}
schema, ok := tool.Tool.OutputSchema.(*jsonschema.Schema)
if !ok {
t.Fatalf("tool %s: OutputSchema is not *jsonschema.Schema (got %T)",
toolName, tool.Tool.OutputSchema)
}
return schema
}
}
t.Fatalf("tool %s is not registered", toolName)
return nil
}

func TestTeamListByCompany(t *testing.T) {
mcpServer := mcpServerMock(t, http.StatusOK, []byte(`{}`))
testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodTeamList.String(), map[string]any{
Expand Down