Skip to content

Commit c3ffc6a

Browse files
test: cover empty-object discussion read requests and modern-text==DTO expectations
Add an explicit {} (fully-empty args) case to TestDiscussionReadRequiredArguments proving the normalizer never synthesizes placeholder owner/repo/discussionNumber values that would let an empty request satisfy SDK required-schema validation and reach GraphQL; verified both protocols reject it pre-handler with zero GraphQL calls (addresses readiness-audit re-flag of already-fixed Copilot review comment 4171698931 in 578e9929). Also record the pending modern-text==structuredContent DTO contract (2026-07-28 protocol) in test expectations; these assertions are intentionally red pending the coordinator's central once-marshal wrapper in pkg/inventory/typed_output.go, which this branch does not yet have. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 2f31d7e commit c3ffc6a

2 files changed

Lines changed: 30 additions & 9 deletions

File tree

‎pkg/github/discussion_notification_contracts_test.go‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,20 @@ func TestDiscussionReadRequiredArguments(t *testing.T) {
4747
}
4848
client := connectCommentVisibilityClient(t, server, protocol)
4949
for _, name := range []string{"get_discussion", "get_discussion_comments"} {
50+
// A wholly empty object must not be normalized into zero-valued
51+
// owner/repo/discussionNumber keys that would satisfy the SDK's
52+
// required-property validation and reach GraphQL.
53+
before := calls
54+
result, err := client.CallTool(context.Background(), &mcp.CallToolParams{Name: name, Arguments: map[string]any{}})
55+
require.NoError(t, err)
56+
require.True(t, result.IsError)
57+
assert.Nil(t, result.StructuredContent)
58+
text := getTextResult(t, result).Text
59+
assert.Contains(t, text, "owner")
60+
assert.Contains(t, text, "repo")
61+
assert.Contains(t, text, "discussionNumber")
62+
assert.Equal(t, before, calls, "empty object must not reach GitHub")
63+
5064
for _, missing := range []string{"owner", "repo", "discussionNumber"} {
5165
args := map[string]any{"owner": "owner", "repo": "repo", "discussionNumber": "1"}
5266
delete(args, missing)
@@ -58,7 +72,7 @@ func TestDiscussionReadRequiredArguments(t *testing.T) {
5872
assert.Contains(t, getTextResult(t, result).Text, missing)
5973
assert.Equal(t, before, calls, "missing required property must not reach GitHub")
6074
}
61-
result, err := client.CallTool(context.Background(), &mcp.CallToolParams{
75+
result, err = client.CallTool(context.Background(), &mcp.CallToolParams{
6276
Name: name, Arguments: map[string]any{"OWNER": "", "REPO": "", "DISCUSSIONNUMBER": 0},
6377
})
6478
require.NoError(t, err)
@@ -100,11 +114,12 @@ func TestDiscussionNotificationEmptyAndNullOutputs(t *testing.T) {
100114
result, err := client.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.tool.Tool.Name, Arguments: tc.args})
101115
require.NoError(t, err)
102116
require.False(t, result.IsError)
103-
assert.Equal(t, tc.legacy, getTextResult(t, result).Text)
104117
if protocol == "2025-11-25" {
118+
assert.Equal(t, tc.legacy, getTextResult(t, result).Text)
105119
assert.Nil(t, result.StructuredContent)
106120
} else {
107121
assert.JSONEq(t, tc.modern, mustMarshalJSON(t, result.StructuredContent))
122+
assert.JSONEq(t, tc.modern, getTextResult(t, result).Text)
108123
resolved, err := tc.tool.Tool.OutputSchema.(*jsonschema.Schema).Resolve(nil)
109124
require.NoError(t, err)
110125
var output any
@@ -142,6 +157,7 @@ func TestDiscussionStructuredSanitizationMatchesLegacy(t *testing.T) {
142157
var legacy, modern map[string]any
143158
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &legacy))
144159
require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, result.StructuredContent)), &modern))
160+
assert.Equal(t, legacy, modern, "modern JSON text and structured DTO must match")
145161
assert.Equal(t, sanitize.PlainText(title), modern["title"])
146162
assert.Equal(t, sanitize.Content(body), modern["body"])
147163
assert.Equal(t, legacy["title"], modern["title"])
@@ -208,17 +224,16 @@ func TestDiscussionEmptyCollections(t *testing.T) {
208224
})
209225
require.NoError(t, err)
210226
require.False(t, result.IsError, "%s", result)
211-
var legacy map[string]any
212-
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &legacy))
213-
assert.Nil(t, legacy[tc.collection], "legacy nil collections retain JSON null")
227+
var text map[string]any
228+
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &text))
214229
if protocol == "2025-11-25" {
230+
assert.Nil(t, text[tc.collection], "legacy nil collections retain JSON null")
215231
assert.Nil(t, result.StructuredContent)
216232
} else {
217233
var modern map[string]any
218234
require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, result.StructuredContent)), &modern))
219235
assert.Equal(t, []any{}, modern[tc.collection])
220-
assert.Equal(t, legacy["pageInfo"], modern["pageInfo"])
221-
assert.Equal(t, legacy["totalCount"], modern["totalCount"])
236+
assert.Equal(t, text, modern, "modern empty collections use the same DTO in both representations")
222237
}
223238
}
224239
})

‎pkg/github/typed_discussion_notification_outputs_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,7 @@ func TestTypedDiscussionNotificationOutputs(t *testing.T) {
224224
result, err := client.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.name, Arguments: tc.args})
225225
require.NoError(t, err)
226226
require.False(t, result.IsError, "%s", result)
227-
require.Len(t, result.Content, 1, "SDK fallback must not duplicate legacy text")
228-
assert.Equal(t, tc.text, getTextResult(t, result).Text, "legacy text must remain byte-equivalent")
227+
require.Len(t, result.Content, 1, "text must not duplicate structured content")
229228
switch tc.name {
230229
case "get_notification_details":
231230
assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelNotificationDetails()), mustMarshalJSON(t, result.Meta["ifc"]))
@@ -235,10 +234,17 @@ func TestTypedDiscussionNotificationOutputs(t *testing.T) {
235234
assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelRepoMetadata(true)), mustMarshalJSON(t, result.Meta["ifc"]))
236235
}
237236
if protocol == "2025-11-25" {
237+
assert.Equal(t, tc.text, getTextResult(t, result).Text, "legacy text must remain byte-equivalent")
238238
assert.Nil(t, result.StructuredContent)
239239
return
240240
}
241241
require.NotNil(t, result.StructuredContent)
242+
if json.Valid([]byte(tc.text)) {
243+
assert.JSONEq(t, mustMarshalJSON(t, result.StructuredContent), getTextResult(t, result).Text,
244+
"modern JSON text must serialize the same compact DTO as structuredContent")
245+
} else {
246+
assert.Equal(t, tc.text, getTextResult(t, result).Text, "plain status and deletion messages remain unchanged")
247+
}
242248
var schema jsonschema.Schema
243249
require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, byName[tc.name].OutputSchema)), &schema))
244250
resolved, err := schema.Resolve(nil)

0 commit comments

Comments
 (0)