From 168c5076a90d31001e1c4d2e612687743851e413 Mon Sep 17 00:00:00 2001 From: Kiran Magic <262980978+kiranmagic7@users.noreply.github.com> Date: Sun, 14 Jun 2026 05:50:21 +0530 Subject: [PATCH 1/4] fix(docs): reject ambiguous tab reads --- internal/cmd/docs_commands_test.go | 17 +++++++++++++++++ internal/cmd/docs_read.go | 3 +++ internal/cmd/mcp_test.go | 14 ++++++++++++++ internal/cmd/mcp_tools.go | 9 +++++++-- 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/internal/cmd/docs_commands_test.go b/internal/cmd/docs_commands_test.go index cccc93087..2e7cf8088 100644 --- a/internal/cmd/docs_commands_test.go +++ b/internal/cmd/docs_commands_test.go @@ -458,6 +458,23 @@ func TestDocsCat_AllTabs_JSON(t *testing.T) { } } +func TestDocsCat_RejectsTabWithAllTabs(t *testing.T) { + t.Parallel() + + docSvc, cleanup := newTabsTestServer(t) + defer cleanup() + + 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) + } +} + func TestDocsCat_Raw(t *testing.T) { t.Parallel() diff --git a/internal/cmd/docs_read.go b/internal/cmd/docs_read.go index 5a5269529..85048fa3a 100644 --- a/internal/cmd/docs_read.go +++ b/internal/cmd/docs_read.go @@ -29,6 +29,9 @@ func (c *DocsCatCmd) Run(ctx context.Context, flags *RootFlags) error { if id == "" { return usage("empty docId") } + if strings.TrimSpace(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/mcp_test.go b/internal/cmd/mcp_test.go index 06b800229..eca632e8b 100644 --- a/internal/cmd/mcp_test.go +++ b/internal/cmd/mcp_test.go @@ -241,6 +241,20 @@ 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) + } +} + 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..bc621f6d6 100644 --- a/internal/cmd/mcp_tools.go +++ b/internal/cmd/mcp_tools.go @@ -174,10 +174,15 @@ 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", "")) + 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 From 4a764bf5232055c7a1be5c92184f837a0c03180f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 13 Jun 2026 20:28:05 -0700 Subject: [PATCH 2/4] test(docs): prove ambiguous reads fail locally --- CHANGELOG.md | 4 ++++ internal/cmd/docs_commands_test.go | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) 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 2e7cf8088..524216152 100644 --- a/internal/cmd/docs_commands_test.go +++ b/internal/cmd/docs_commands_test.go @@ -461,8 +461,21 @@ func TestDocsCat_AllTabs_JSON(t *testing.T) { func TestDocsCat_RejectsTabWithAllTabs(t *testing.T) { t.Parallel() - docSvc, cleanup := newTabsTestServer(t) - defer cleanup() + 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") { @@ -473,6 +486,9 @@ func TestDocsCat_RejectsTabWithAllTabs(t *testing.T) { 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) } + if requests != 0 { + t.Fatalf("Docs API requests = %d, want 0", requests) + } } func TestDocsCat_Raw(t *testing.T) { From 2aaac77759fb3eddfa9121cd78bf76e9395468a6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 13 Jun 2026 20:49:05 -0700 Subject: [PATCH 3/4] fix(docs): reject blank tab selectors locally --- internal/cmd/docs_commands_test.go | 10 ++++++++++ internal/cmd/docs_read.go | 7 ++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/cmd/docs_commands_test.go b/internal/cmd/docs_commands_test.go index 524216152..506a423ea 100644 --- a/internal/cmd/docs_commands_test.go +++ b/internal/cmd/docs_commands_test.go @@ -486,6 +486,16 @@ func TestDocsCat_RejectsTabWithAllTabs(t *testing.T) { 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) + } if requests != 0 { t.Fatalf("Docs API requests = %d, want 0", requests) } diff --git a/internal/cmd/docs_read.go b/internal/cmd/docs_read.go index 85048fa3a..09e259d76 100644 --- a/internal/cmd/docs_read.go +++ b/internal/cmd/docs_read.go @@ -29,7 +29,12 @@ func (c *DocsCatCmd) Run(ctx context.Context, flags *RootFlags) error { if id == "" { return usage("empty docId") } - if strings.TrimSpace(c.Tab) != "" && c.AllTabs { + tab := strings.TrimSpace(c.Tab) + if c.Tab != "" && 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") } From ecfdaebda78bd2a1e156bf63e3d9be696ba43d22 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 13 Jun 2026 20:52:56 -0700 Subject: [PATCH 4/4] fix(docs): detect explicit empty tab flags --- internal/cmd/docs_commands_test.go | 10 ++++++++++ internal/cmd/docs_read.go | 6 ++++-- internal/cmd/docs_validation_more_test.go | 4 ++-- internal/cmd/mcp_test.go | 10 ++++++++++ internal/cmd/mcp_tools.go | 4 ++++ 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/internal/cmd/docs_commands_test.go b/internal/cmd/docs_commands_test.go index 506a423ea..7c31d3dd1 100644 --- a/internal/cmd/docs_commands_test.go +++ b/internal/cmd/docs_commands_test.go @@ -496,6 +496,16 @@ func TestDocsCat_RejectsTabWithAllTabs(t *testing.T) { 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) } diff --git a/internal/cmd/docs_read.go b/internal/cmd/docs_read.go index 09e259d76..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,13 +25,14 @@ 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 c.Tab != "" && tab == "" { + if tabProvided && tab == "" { return usage("--tab cannot be empty") } c.Tab = tab 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 eca632e8b..5f953877d 100644 --- a/internal/cmd/mcp_test.go +++ b/internal/cmd/mcp_test.go @@ -253,6 +253,16 @@ func TestMCPDocsGetRejectsTabWithAllTabs(t *testing.T) { 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) { diff --git a/internal/cmd/mcp_tools.go b/internal/cmd/mcp_tools.go index bc621f6d6..6ace23d44 100644 --- a/internal/cmd/mcp_tools.go +++ b/internal/cmd/mcp_tools.go @@ -175,6 +175,10 @@ func mcpDocsGetTool() mcpToolSpec { } args := []string{"docs", "cat", "--max-bytes", strconv.Itoa(clampMCPInt(req.GetInt("max_bytes", 2000000), 0, 20_000_000))} 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")