Skip to content

Commit 8c9d14c

Browse files
committed
fix: correct API paths, drop bad params, tighten MCP arg handling
reports: fix family path mappings (chat, email, happiness/ratings, user) teams: remove unsupported pageSize query param mcp: reject empty required positional args, restrict StructuredContent to objects
1 parent 3c9d094 commit 8c9d14c

File tree

6 files changed

+83
-9
lines changed

6 files changed

+83
-9
lines changed

internal/cmd/mcp_execute.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ func (r *mcpCommandRunner) execute(ctx context.Context, spec mcpToolSpec, args m
7979
}},
8080
}
8181

82-
if parsed, ok := parseJSONValue(stdoutText); ok {
83-
result.StructuredContent = parsed
82+
if structured, ok := parseStructuredContent(stdoutText); ok {
83+
result.StructuredContent = structured
8484
}
8585

8686
return result
@@ -130,6 +130,13 @@ func (r *mcpCommandRunner) buildInvocation(spec mcpToolSpec, args map[string]jso
130130
if err != nil {
131131
return nil, "", fmt.Errorf("invalid %s: %w", arg.Property, err)
132132
}
133+
value = strings.TrimSpace(value)
134+
if value == "" {
135+
if arg.Required {
136+
return nil, "", fmt.Errorf("missing required argument: %s", arg.Property)
137+
}
138+
continue
139+
}
133140
argv = append(argv, value)
134141
}
135142

@@ -230,6 +237,18 @@ func parseJSONValue(value string) (any, bool) {
230237
return parsed, true
231238
}
232239

240+
func parseStructuredContent(value string) (map[string]any, bool) {
241+
parsed, ok := parseJSONValue(value)
242+
if !ok {
243+
return nil, false
244+
}
245+
obj, ok := parsed.(map[string]any)
246+
if !ok {
247+
return nil, false
248+
}
249+
return obj, true
250+
}
251+
233252
func setEnvVar(env []string, key, value string) []string {
234253
prefix := key + "="
235254
out := make([]string, 0, len(env)+1)

internal/cmd/mcp_execute_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,28 @@ func TestMCPBuildInvocation_ForwardsGlobalConfigAndDebug(t *testing.T) {
8787
"--format", "json",
8888
}, argv)
8989
}
90+
91+
func TestMCPBuildInvocation_RejectsEmptyRequiredPositionalArgument(t *testing.T) {
92+
runner := &mcpCommandRunner{defaultOutputMode: mcpOutputJSON}
93+
spec := mcpToolSpec{
94+
CommandPath: []string{"inbox", "teams", "members"},
95+
PositionalArgs: []mcpPositionalArgSpec{
96+
{Name: "team-id", Property: "team_id", Required: true},
97+
},
98+
}
99+
100+
_, _, err := runner.buildInvocation(spec, map[string]json.RawMessage{
101+
"team_id": json.RawMessage(`""`),
102+
})
103+
require.Error(t, err)
104+
assert.Contains(t, err.Error(), "missing required argument: team_id")
105+
}
106+
107+
func TestParseStructuredContent_ObjectOnly(t *testing.T) {
108+
obj, ok := parseStructuredContent(`{"ok":true}`)
109+
require.True(t, ok)
110+
assert.Equal(t, true, obj["ok"])
111+
112+
_, ok = parseStructuredContent(`[{"id":1}]`)
113+
assert.False(t, ok)
114+
}

internal/cmd/reports.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ import (
1313
)
1414

1515
var reportFamilies = map[string]string{
16-
"chats": "chats",
16+
"chats": "chat",
1717
"company": "company",
1818
"conversations": "conversations",
1919
"customers": "customers",
2020
"docs": "docs",
21-
"email": "emails",
21+
"email": "email",
2222
"productivity": "productivity",
23-
"ratings": "ratings",
24-
"users": "users",
23+
"ratings": "happiness/ratings",
24+
"users": "user",
2525
}
2626

2727
func newReportsCmd() *cobra.Command {

internal/cmd/reports_test.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestReportsCompany(t *testing.T) {
4242
func TestReportsEmailPathMapping(t *testing.T) {
4343
mock := &mockClient{
4444
GetReportFn: func(ctx context.Context, family string, params url.Values) (json.RawMessage, error) {
45-
assert.Equal(t, "emails", family)
45+
assert.Equal(t, "email", family)
4646
return json.RawMessage(`{"ok":true}`), nil
4747
},
4848
}
@@ -53,6 +53,34 @@ func TestReportsEmailPathMapping(t *testing.T) {
5353
require.NoError(t, rootCmd.Execute())
5454
}
5555

56+
func TestReportsRatingsPathMapping(t *testing.T) {
57+
mock := &mockClient{
58+
GetReportFn: func(ctx context.Context, family string, params url.Values) (json.RawMessage, error) {
59+
assert.Equal(t, "happiness/ratings", family)
60+
return json.RawMessage(`{"ok":true}`), nil
61+
},
62+
}
63+
setupTest(mock)
64+
defer func() { output.Out = os.Stdout }()
65+
66+
rootCmd.SetArgs([]string{"inbox", "reports", "ratings"})
67+
require.NoError(t, rootCmd.Execute())
68+
}
69+
70+
func TestReportsUsersPathMapping(t *testing.T) {
71+
mock := &mockClient{
72+
GetReportFn: func(ctx context.Context, family string, params url.Values) (json.RawMessage, error) {
73+
assert.Equal(t, "user", family)
74+
return json.RawMessage(`{"ok":true}`), nil
75+
},
76+
}
77+
setupTest(mock)
78+
defer func() { output.Out = os.Stdout }()
79+
80+
rootCmd.SetArgs([]string{"inbox", "reports", "users"})
81+
require.NoError(t, rootCmd.Execute())
82+
}
83+
5684
func TestReportsFamiliesExposed(t *testing.T) {
5785
reportsCmd := newReportsCmd()
5886
names := make(map[string]struct{})

internal/cmd/teams.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ func teamsListCmd() *cobra.Command {
3939
ctx := context.Background()
4040
params := url.Values{}
4141
params.Set("page", strconv.Itoa(page))
42-
params.Set("pageSize", strconv.Itoa(perPage))
4342

4443
if isJSON() {
4544
items, _, err := api.PaginateAll(ctx, apiClient.ListTeams, params, "teams", noPaginate)
@@ -85,7 +84,6 @@ func teamsMembersCmd() *cobra.Command {
8584
ctx := context.Background()
8685
params := url.Values{}
8786
params.Set("page", strconv.Itoa(page))
88-
params.Set("pageSize", strconv.Itoa(perPage))
8987

9088
fetch := func(ctx context.Context, p url.Values) (json.RawMessage, error) {
9189
return apiClient.ListTeamMembers(ctx, args[0], p)

internal/cmd/teams_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
func TestTeamsList(t *testing.T) {
1616
mock := &mockClient{
1717
ListTeamsFn: func(ctx context.Context, params url.Values) (json.RawMessage, error) {
18+
assert.Equal(t, "1", params.Get("page"))
19+
assert.Empty(t, params.Get("pageSize"))
1820
return halJSON("teams", `[{"id":1,"name":"Support"}]`), nil
1921
},
2022
}
@@ -30,6 +32,8 @@ func TestTeamsMembers(t *testing.T) {
3032
mock := &mockClient{
3133
ListTeamMembersFn: func(ctx context.Context, id string, params url.Values) (json.RawMessage, error) {
3234
assert.Equal(t, "1", id)
35+
assert.Equal(t, "1", params.Get("page"))
36+
assert.Empty(t, params.Get("pageSize"))
3337
return halJSON("users", `[{"id":2,"firstName":"Agent","lastName":"One","email":"agent@test.com","role":"user"}]`), nil
3438
},
3539
}

0 commit comments

Comments
 (0)