diff --git a/CHANGELOG.md b/CHANGELOG.md index 02be656fa..099f00490 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## 0.26.1 - Unreleased +### Fixed + +- Docs: reject ambiguous `docs cat --tab ... --all-tabs` and MCP `docs_get` requests before contacting the Docs API. (#801) — thanks @kiranmagic7. + ## 0.26.0 - 2026-06-14 ### Added diff --git a/internal/cmd/docs_commands_test.go b/internal/cmd/docs_commands_test.go index cccc93087..7c31d3dd1 100644 --- a/internal/cmd/docs_commands_test.go +++ b/internal/cmd/docs_commands_test.go @@ -458,6 +458,59 @@ func TestDocsCat_AllTabs_JSON(t *testing.T) { } } +func TestDocsCat_RejectsTabWithAllTabs(t *testing.T) { + t.Parallel() + + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests++ + http.Error(w, "unexpected Docs API request", http.StatusInternalServerError) + })) + defer srv.Close() + + docSvc, err := docs.NewService(context.Background(), + option.WithoutAuthentication(), + option.WithHTTPClient(srv.Client()), + option.WithEndpoint(srv.URL+"/"), + ) + if err != nil { + t.Fatalf("NewDocsService: %v", err) + } + + result := runDocsCatCommand(t, docSvc, []string{"doc1", "--tab", "Overview", "--all-tabs"}, false) + if result.err == nil || !strings.Contains(result.err.Error(), "--tab and --all-tabs cannot be used together") { + t.Fatalf("expected tab/all-tabs usage error, got: %v", result.err) + } + + rawResult := runDocsCatCommand(t, docSvc, []string{"doc1", "--raw", "--tab", "Overview", "--all-tabs"}, false) + if rawResult.err == nil || !strings.Contains(rawResult.err.Error(), "--tab and --all-tabs cannot be used together") { + t.Fatalf("expected raw tab/all-tabs usage error, got: %v", rawResult.err) + } + + emptyTabResult := runDocsCatCommand(t, docSvc, []string{"doc1", "--tab", " "}, false) + if emptyTabResult.err == nil || !strings.Contains(emptyTabResult.err.Error(), "--tab cannot be empty") { + t.Fatalf("expected empty tab usage error, got: %v", emptyTabResult.err) + } + + emptyRawTabResult := runDocsCatCommand(t, docSvc, []string{"doc1", "--raw", "--tab", " ", "--all-tabs"}, false) + if emptyRawTabResult.err == nil || !strings.Contains(emptyRawTabResult.err.Error(), "--tab cannot be empty") { + t.Fatalf("expected raw empty tab usage error, got: %v", emptyRawTabResult.err) + } + + explicitEmptyTabResult := runDocsCatCommand(t, docSvc, []string{"doc1", "--tab="}, false) + if explicitEmptyTabResult.err == nil || !strings.Contains(explicitEmptyTabResult.err.Error(), "--tab cannot be empty") { + t.Fatalf("expected explicit empty tab usage error, got: %v", explicitEmptyTabResult.err) + } + + explicitEmptyRawTabResult := runDocsCatCommand(t, docSvc, []string{"doc1", "--raw", "--tab=", "--all-tabs"}, false) + if explicitEmptyRawTabResult.err == nil || !strings.Contains(explicitEmptyRawTabResult.err.Error(), "--tab cannot be empty") { + t.Fatalf("expected raw explicit empty tab usage error, got: %v", explicitEmptyRawTabResult.err) + } + if requests != 0 { + t.Fatalf("Docs API requests = %d, want 0", requests) + } +} + func TestDocsCat_Raw(t *testing.T) { t.Parallel() diff --git a/internal/cmd/docs_read.go b/internal/cmd/docs_read.go index 5a5269529..4a8acd6e7 100644 --- a/internal/cmd/docs_read.go +++ b/internal/cmd/docs_read.go @@ -9,6 +9,7 @@ import ( "io" "strings" + "github.com/alecthomas/kong" "google.golang.org/api/docs/v1" "github.com/steipete/gogcli/internal/outfmt" @@ -24,11 +25,20 @@ type DocsCatCmd struct { Numbered bool `name:"numbered" short:"N" help:"Prefix each paragraph with its number"` } -func (c *DocsCatCmd) Run(ctx context.Context, flags *RootFlags) error { +func (c *DocsCatCmd) Run(ctx context.Context, kctx *kong.Context, flags *RootFlags) error { id := strings.TrimSpace(c.DocID) if id == "" { return usage("empty docId") } + tabProvided := flagProvided(kctx, "tab") || c.Tab != "" + tab := strings.TrimSpace(c.Tab) + if tabProvided && tab == "" { + return usage("--tab cannot be empty") + } + c.Tab = tab + if c.Tab != "" && c.AllTabs { + return usage("--tab and --all-tabs cannot be used together") + } svc, err := requireDocsService(ctx, flags) if err != nil { diff --git a/internal/cmd/docs_validation_more_test.go b/internal/cmd/docs_validation_more_test.go index faf14f9b8..c7f40cb98 100644 --- a/internal/cmd/docs_validation_more_test.go +++ b/internal/cmd/docs_validation_more_test.go @@ -68,7 +68,7 @@ func TestDocsCreateCat_ValidationErrors(t *testing.T) { if err := (&DocsCreateCmd{}).Run(ctx, flags); err == nil { t.Fatalf("expected missing title error") } - if err := (&DocsCatCmd{}).Run(ctx, flags); err == nil { + if err := (&DocsCatCmd{}).Run(ctx, nil, flags); err == nil { t.Fatalf("expected missing docId error") } } @@ -113,7 +113,7 @@ func TestDocsCat_JSON_EmptyDoc(t *testing.T) { ctx := withDocsTestService(newCmdRuntimeJSONOutputContext(t, &output, io.Discard), svc) flags := &RootFlags{Account: "a@b.com"} - if err := (&DocsCatCmd{DocID: "doc1"}).Run(ctx, flags); err != nil { + if err := (&DocsCatCmd{DocID: "doc1"}).Run(ctx, nil, flags); err != nil { t.Fatalf("cat: %v", err) } if !strings.Contains(output.String(), "\"text\"") { diff --git a/internal/cmd/mcp_test.go b/internal/cmd/mcp_test.go index 06b800229..5f953877d 100644 --- a/internal/cmd/mcp_test.go +++ b/internal/cmd/mcp_test.go @@ -241,6 +241,30 @@ func TestMCPDocsWriteRejectsNeitherAppendNorReplace(t *testing.T) { } } +func TestMCPDocsGetRejectsTabWithAllTabs(t *testing.T) { + tool := findMCPTool(t, "docs_get") + _, err := tool.BuildArgs(mcp.CallToolRequest{Params: mcp.CallToolParams{ + Arguments: map[string]any{ + "document_id": "doc1", + "tab": "Overview", + "all_tabs": true, + }, + }}) + if err == nil || !strings.Contains(err.Error(), "mutually exclusive") { + t.Fatalf("expected tab/all_tabs error, got %v", err) + } + + _, err = tool.BuildArgs(mcp.CallToolRequest{Params: mcp.CallToolParams{ + Arguments: map[string]any{ + "document_id": "doc1", + "tab": "", + }, + }}) + if err == nil || !strings.Contains(err.Error(), "tab cannot be empty") { + t.Fatalf("expected empty tab error, got %v", err) + } +} + func TestMCPSheetsUpdateRejectsFileExpansion(t *testing.T) { tool := findMCPTool(t, "sheets_update_range") _, err := tool.BuildArgs(mcp.CallToolRequest{Params: mcp.CallToolParams{ diff --git a/internal/cmd/mcp_tools.go b/internal/cmd/mcp_tools.go index d3f4a8c92..6ace23d44 100644 --- a/internal/cmd/mcp_tools.go +++ b/internal/cmd/mcp_tools.go @@ -174,10 +174,19 @@ func mcpDocsGetTool() mcpToolSpec { return nil, err } args := []string{"docs", "cat", "--max-bytes", strconv.Itoa(clampMCPInt(req.GetInt("max_bytes", 2000000), 0, 20_000_000))} - if tab := strings.TrimSpace(req.GetString("tab", "")); tab != "" { + tab := strings.TrimSpace(req.GetString("tab", "")) + _, tabProvided := req.GetArguments()["tab"] + if tabProvided && tab == "" { + return nil, fmt.Errorf("tab cannot be empty") + } + allTabs := req.GetBool("all_tabs", false) + if tab != "" && allTabs { + return nil, fmt.Errorf("tab and all_tabs are mutually exclusive") + } + if tab != "" { args = append(args, "--tab", tab) } - if req.GetBool("all_tabs", false) { + if allTabs { args = append(args, "--all-tabs") } return append(args, "--", docID), nil