Fix: team tools fail output-schema validation on every call - #458
Merged
Conversation
`twprojects-get_team` and `twprojects-list_teams` returned structured content
that no validating client would accept: the schema declared `deletedDate` as
`object|null` while the value on the wire was a string. Clients that validate
discarded every response; the rest never noticed.
`twapi.OptionalDateTime` is defined over `time.Time`, so `jsonschema.For`
describes it by `time.Time`'s unexported fields, while its `MarshalJSON` writes
an RFC3339 string. Register the override in `helpers.WithDateTypeSchema`
alongside `twapi.Date` rather than inline in `teams.go`: a response picks the
type up transitively, and `SearchResponse` sideloads `Team`, so
`twprojects-search` carried the same defect and is fixed by the same change.
twapi-go-sdk v1.21.4 makes the zero value marshal as `null`, which covers a live
team on `get_team`. It does not cover a deleted one, and it does not touch
`list_teams` at all — that tool streams the raw API body per the `list_*`
contract, so `MarshalJSON` never runs and the API's literal `deletedDate: ""`
reaches the client unchanged. The schema override is what fixes those.
Neither the server nor `testutil.ExecuteToolRequest` validates structured
content against the output schema, so the new test pulls the tool's published
schema and validates the result by hand — driving the tool alone passes against
the broken code. The existing team tests reply `{}`, which leaves the field nil
and encodes as `null`, so they could never have caught this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
twprojects-get_teamandtwprojects-list_teamsreturned structured content that no validating client would accept: the schema declareddeletedDateasobject|nullwhile the value on the wire was a string. Clients that validate discarded every response; the rest never noticed.twapi.OptionalDateTimeis defined overtime.Time, sojsonschema.Fordescribes it bytime.Time's unexported fields, while itsMarshalJSONwrites an RFC3339 string. Register the override inhelpers.WithDateTypeSchemaalongsidetwapi.Daterather than inline inteams.go: a response picks the type up transitively, andSearchResponsesideloadsTeam, sotwprojects-searchcarried the same defect and is fixed by the same change.twapi-go-sdk v1.21.4 makes the zero value marshal as
null, which covers a live team onget_team. It does not cover a deleted one, and it does not touchlist_teamsat all — that tool streams the raw API body per thelist_*contract, soMarshalJSONnever runs and the API's literaldeletedDate: ""reaches the client unchanged. The schema override is what fixes those.Neither the server nor
testutil.ExecuteToolRequestvalidates structured content against the output schema, so the new test pulls the tool's published schema and validates the result by hand — driving the tool alone passes against the broken code. The existing team tests reply{}, which leaves the field nil and encodes asnull, so they could never have caught this.Type of Change
Testing
go test -v ./...)Checklist