From 7b412446350aed5582634ce87148ce5be2adb31f Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 22:19:14 +0300 Subject: [PATCH 01/12] feat: --raw + --params for programmatic API-shape output (#188) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a programmatic mode for scripting Google Workspace via gws: * --raw emits the unmodified Google API response JSON (no field renaming, no base64 decoding, no header collapsing). Default ergonomic output is unchanged when the flag is not set. * --params passes a JSON object whose keys map directly to the underlying API request parameters. Keys override the equivalent CLI flags (params win) — documented in help/README. Under --all, raw mode concatenates the top-level list field across pages (messages / spaces / members) and drops nextPageToken from the final output. Coverage in this PR (per issue #188): gmail list (users.messages.list — switches to messages.list under --raw to match the API shape) gmail thread (users.threads.get — payload tree, headers as {name,value} arrays, base64 body.data, internalDate, labelIds, snippet) chat spaces list (spaces.list) chat members list (spaces.members.list) chat messages list (spaces.messages.list) people get (people.get) New noun-verb command paths (chat spaces|messages|members list, gws people get) live alongside the existing ergonomic commands; the existing chat list / chat messages / chat members / contacts get also gain --raw + --params for an easy upgrade path. No existing flags or output shapes change. Tests cover --raw output keys/structure for all six commands by marshaling the SDK response structs and asserting against Google's documented shape, plus --params parsing edge cases. All existing tests continue to pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 36 +++++ cmd/chat.go | 40 ++++- cmd/chat_raw.go | 362 +++++++++++++++++++++++++++++++++++++++++++ cmd/commands_test.go | 30 ++++ cmd/contacts.go | 25 ++- cmd/gmail.go | 147 ++++++++++++++++++ cmd/people.go | 104 +++++++++++++ cmd/raw.go | 164 ++++++++++++++++++++ cmd/raw_test.go | 346 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 1249 insertions(+), 5 deletions(-) create mode 100644 cmd/chat_raw.go create mode 100644 cmd/people.go create mode 100644 cmd/raw.go create mode 100644 cmd/raw_test.go diff --git a/README.md b/README.md index 4ac5727..77e3182 100644 --- a/README.md +++ b/README.md @@ -456,6 +456,42 @@ $ gws calendar events --days 1 } ``` +### Programmatic mode: `--raw` and `--params` + +Several list/get commands accept two extra flags for scripting: + +- `--raw` emits the unmodified Google API response JSON (no field renaming, + no base64 decoding, no header collapsing). Default ergonomic output is + untouched when the flag is not set. +- `--params ` accepts a JSON object whose keys map directly to the + underlying API request parameters. Keys supplied here **override** the + equivalent CLI flags (params win). + +Under `--all`, raw mode concatenates the top-level list field across pages +(messages / spaces / members) and drops `nextPageToken` from the final +output. + +Supported in this release: + +| Command | Wraps | +|---|---| +| `gmail list` | `users.messages.list` (under `--raw`) | +| `gmail thread ` | `users.threads.get` | +| `chat spaces list` | `spaces.list` | +| `chat members list` | `spaces.members.list` | +| `chat messages list` | `spaces.messages.list` | +| `people get` | `people.get` | + +Examples: + +```bash +gws gmail list --query "in:sent" --max 5 --raw +gws gmail thread 18abc --raw +gws chat spaces list --params '{"pageSize":50}' --all --raw +gws chat messages list --params '{"parent":"spaces/AAA","pageSize":50,"filter":"createTime > \"2025-01-01T00:00:00Z\""}' --all --raw +gws people get --params '{"resourceName":"people/me","personFields":"emailAddresses"}' --raw +``` + ## Development ### Project Layout diff --git a/cmd/chat.go b/cmd/chat.go index b74f6a6..8fef735 100644 --- a/cmd/chat.go +++ b/cmd/chat.go @@ -581,6 +581,14 @@ func runChatList(cmd *cobra.Command, args []string) error { filter, _ := cmd.Flags().GetString("filter") pageSize, _ := cmd.Flags().GetInt64("page-size") maxResults, _ := cmd.Flags().GetInt64("max") + fetchAll := false + if f := cmd.Flags().Lookup("all"); f != nil { + fetchAll, _ = cmd.Flags().GetBool("all") + } + + if isRaw(cmd) { + return runChatListRaw(cmd, svc, filter, pageSize, maxResults, fetchAll) + } var results []map[string]interface{} var pageToken string @@ -636,7 +644,10 @@ func runChatMessages(cmd *cobra.Command, args []string) error { return p.PrintError(err) } - spaceName := ensureSpaceName(args[0]) + spaceName := "" + if len(args) > 0 { + spaceName = ensureSpaceName(args[0]) + } maxResults, _ := cmd.Flags().GetInt64("max") filter, _ := cmd.Flags().GetString("filter") orderBy, _ := cmd.Flags().GetString("order-by") @@ -644,6 +655,17 @@ func runChatMessages(cmd *cobra.Command, args []string) error { after, _ := cmd.Flags().GetString("after") before, _ := cmd.Flags().GetString("before") resolveSenders, _ := cmd.Flags().GetBool("resolve-senders") + fetchAll := false + if f := cmd.Flags().Lookup("all"); f != nil { + fetchAll, _ = cmd.Flags().GetBool("all") + } + + if isRaw(cmd) { + return runChatMessagesRaw(cmd, svc, spaceName, maxResults, filter, orderBy, showDeleted, fetchAll) + } + if spaceName == "" { + return p.PrintError(errors.New("chat messages: a space id is required")) + } // Build filter from --after/--before flags, combining with --filter var filterParts []string @@ -1027,11 +1049,25 @@ func runChatMembers(cmd *cobra.Command, args []string) error { return p.PrintError(err) } - spaceName := ensureSpaceName(args[0]) + spaceName := "" + if len(args) > 0 { + spaceName = ensureSpaceName(args[0]) + } maxResults, _ := cmd.Flags().GetInt64("max") filter, _ := cmd.Flags().GetString("filter") showGroups, _ := cmd.Flags().GetBool("show-groups") showInvited, _ := cmd.Flags().GetBool("show-invited") + fetchAll := false + if f := cmd.Flags().Lookup("all"); f != nil { + fetchAll, _ = cmd.Flags().GetBool("all") + } + + if isRaw(cmd) { + return runChatMembersRaw(cmd, svc, spaceName, maxResults, filter, showGroups, showInvited, fetchAll) + } + if spaceName == "" { + return p.PrintError(errors.New("chat members: a space id is required")) + } // Page size per request (Google caps at 100) pageSize := maxResults diff --git a/cmd/chat_raw.go b/cmd/chat_raw.go new file mode 100644 index 0000000..79cff0b --- /dev/null +++ b/cmd/chat_raw.go @@ -0,0 +1,362 @@ +package cmd + +// `--raw` + `--params` paths for the Chat list endpoints documented in #188: +// * spaces.list (gws chat spaces list / gws chat list --raw) +// * spaces.members.list (gws chat members list) +// * spaces.messages.list (gws chat messages list) +// +// Each runner emits the SDK response struct as JSON — Google's API shape: +// `{"spaces":[...],"nextPageToken":"..."}`, etc. With --all, the list field +// is concatenated across pages and nextPageToken is dropped. + +import ( + "context" + "errors" + "fmt" + + "github.com/omriariav/workspace-cli/internal/client" + "github.com/spf13/cobra" + "google.golang.org/api/chat/v1" +) + +// chatSpacesCmd is a noun-style parent so callers can write +// `gws chat spaces list` per the API reference. `gws chat list` keeps +// working with its existing ergonomic output. +var chatSpacesCmd = &cobra.Command{ + Use: "spaces", + Short: "Chat spaces (API-shape friendly subcommands)", +} + +var chatSpacesListCmd = &cobra.Command{ + Use: "list", + Short: "List Chat spaces", + Long: `List Chat spaces. Supports --raw and --params for programmatic use. + +--raw emits the unmodified spaces.list response JSON. +--params overrides equivalent flags; for example: + gws chat spaces list --params '{"pageSize":50,"filter":"spaceType = \"DIRECT_MESSAGE\""}' --raw --all`, + RunE: runChatList, +} + +var chatMessagesListCmd = &cobra.Command{ + Use: "list", + Short: "List messages in a Chat space", + Long: `List messages in a Chat space. + +Either pass as a positional argument or set "parent" via --params. +Supports --raw and --params for programmatic use. Example: + gws chat messages list --params '{"parent":"spaces/AAA","pageSize":50,"filter":"createTime > \"2025-01-01T00:00:00Z\""}' --raw --all`, + Args: cobra.MaximumNArgs(1), + RunE: runChatMessages, +} + +var chatMembersListCmd = &cobra.Command{ + Use: "list", + Short: "List members of a Chat space", + Long: `List members of a Chat space. + +Either pass as a positional argument or set "parent" via --params. +Supports --raw and --params for programmatic use.`, + Args: cobra.MaximumNArgs(1), + RunE: runChatMembers, +} + +func init() { + chatCmd.AddCommand(chatSpacesCmd) + chatSpacesCmd.AddCommand(chatSpacesListCmd) + chatMessagesCmd.AddCommand(chatMessagesListCmd) + chatMembersCmd.AddCommand(chatMembersListCmd) + + // Mirror the underlying flag surface for the new paths. + chatSpacesListCmd.Flags().String("filter", "", "Filter spaces (e.g. 'spaceType = \"DIRECT_MESSAGE\"')") + chatSpacesListCmd.Flags().Int64("page-size", 100, "Number of spaces per page") + chatSpacesListCmd.Flags().Int64("max", 0, "Maximum number of spaces to return (0 = all)") + chatSpacesListCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") + addRawParamsFlags(chatSpacesListCmd) + + chatMessagesListCmd.Flags().Int64("max", 25, "Maximum number of messages to return") + chatMessagesListCmd.Flags().String("filter", "", "Filter messages (e.g. 'createTime > \"2024-01-01T00:00:00Z\"')") + chatMessagesListCmd.Flags().String("order-by", "", "Order messages (e.g. 'createTime DESC')") + chatMessagesListCmd.Flags().Bool("show-deleted", false, "Include deleted messages") + chatMessagesListCmd.Flags().String("after", "", "Show messages after this time (RFC3339)") + chatMessagesListCmd.Flags().String("before", "", "Show messages before this time (RFC3339)") + chatMessagesListCmd.Flags().Bool("resolve-senders", false, "Resolve sender display names (non-raw mode only)") + chatMessagesListCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") + addRawParamsFlags(chatMessagesListCmd) + + chatMembersListCmd.Flags().Int64("max", 100, "Maximum number of members to return") + chatMembersListCmd.Flags().String("filter", "", "Filter members (e.g. 'member.type = \"HUMAN\"')") + chatMembersListCmd.Flags().Bool("show-groups", false, "Include Google Group memberships") + chatMembersListCmd.Flags().Bool("show-invited", false, "Include invited memberships") + chatMembersListCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") + addRawParamsFlags(chatMembersListCmd) + + // Also expose --raw + --params on the existing leaf commands so + // scripts that already use them have a smooth upgrade path. + addRawParamsFlags(chatListCmd) + addRawParamsFlags(chatMessagesCmd) + addRawParamsFlags(chatMembersCmd) + chatListCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") + chatMessagesCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") + chatMembersCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") +} + +// runChatListRaw implements `gws chat spaces list --raw` (and `gws chat list --raw`). +func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSize, maxResults int64, fetchAll bool) error { + params, perr := parseParams(cmd) + if perr != nil { + return perr + } + + if v, ok := paramString(params, "filter"); ok { + filter = v + } + if v, ok := paramInt64(params, "pageSize"); ok { + pageSize = v + } + pageToken, _ := paramString(params, "pageToken") + + if pageSize <= 0 { + pageSize = 100 + } + + var aggregated *chat.ListSpacesResponse + for { + call := svc.Spaces.List().PageSize(pageSize) + if filter != "" { + call = call.Filter(filter) + } + if pageToken != "" { + call = call.PageToken(pageToken) + } + + resp, err := call.Do() + if err != nil { + return fmt.Errorf("failed to list spaces: %w", err) + } + + if aggregated == nil { + aggregated = resp + } else { + aggregated.Spaces = append(aggregated.Spaces, resp.Spaces...) + aggregated.NextPageToken = resp.NextPageToken + } + + // Stop conditions. + if resp.NextPageToken == "" { + break + } + if !fetchAll { + if maxResults > 0 && int64(len(aggregated.Spaces)) >= maxResults { + break + } + // Without --all we only want one page unless caller paged via --params. + break + } + if maxResults > 0 && int64(len(aggregated.Spaces)) >= maxResults { + break + } + pageToken = resp.NextPageToken + } + + if aggregated == nil { + aggregated = &chat.ListSpacesResponse{} + } + if maxResults > 0 && int64(len(aggregated.Spaces)) > maxResults { + aggregated.Spaces = aggregated.Spaces[:maxResults] + } + if fetchAll { + aggregated.NextPageToken = "" + } + return printRaw(aggregated) +} + +// runChatMessagesRaw implements `gws chat messages list --raw`. +func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter, orderBy string, showDeleted, fetchAll bool) error { + params, perr := parseParams(cmd) + if perr != nil { + return perr + } + + if v, ok := paramString(params, "parent"); ok && v != "" { + spaceName = v + } + if spaceName == "" { + return errors.New("chat messages list: a space name is required (positional arg or --params parent)") + } + pageSize := int64(0) + if v, ok := paramInt64(params, "pageSize"); ok { + pageSize = v + } + if v, ok := paramString(params, "filter"); ok { + filter = v + } + if v, ok := paramString(params, "orderBy"); ok { + orderBy = v + } + if v, ok := paramBool(params, "showDeleted"); ok { + showDeleted = v + } + pageToken, _ := paramString(params, "pageToken") + + var aggregated *chat.ListMessagesResponse + for { + thisPage := pageSize + if thisPage <= 0 { + thisPage = maxResults + if thisPage <= 0 || thisPage > 1000 { + thisPage = 1000 + } + } + + call := svc.Spaces.Messages.List(spaceName).PageSize(thisPage) + if filter != "" { + call = call.Filter(filter) + } + if orderBy != "" { + call = call.OrderBy(orderBy) + } + if showDeleted { + call = call.ShowDeleted(true) + } + if pageToken != "" { + call = call.PageToken(pageToken) + } + + resp, err := call.Do() + if err != nil { + return fmt.Errorf("failed to list messages: %w", err) + } + + if aggregated == nil { + aggregated = resp + } else { + aggregated.Messages = append(aggregated.Messages, resp.Messages...) + aggregated.NextPageToken = resp.NextPageToken + } + + if resp.NextPageToken == "" { + break + } + if !fetchAll { + break + } + if maxResults > 0 && int64(len(aggregated.Messages)) >= maxResults { + break + } + pageToken = resp.NextPageToken + } + + if aggregated == nil { + aggregated = &chat.ListMessagesResponse{} + } + if maxResults > 0 && int64(len(aggregated.Messages)) > maxResults { + aggregated.Messages = aggregated.Messages[:maxResults] + } + if fetchAll { + aggregated.NextPageToken = "" + } + return printRaw(aggregated) +} + +// runChatMembersRaw implements `gws chat members list --raw`. +func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter string, showGroups, showInvited, fetchAll bool) error { + params, perr := parseParams(cmd) + if perr != nil { + return perr + } + + if v, ok := paramString(params, "parent"); ok && v != "" { + spaceName = v + } + if spaceName == "" { + return errors.New("chat members list: a space name is required (positional arg or --params parent)") + } + pageSize := int64(0) + if v, ok := paramInt64(params, "pageSize"); ok { + pageSize = v + } + if v, ok := paramString(params, "filter"); ok { + filter = v + } + if v, ok := paramBool(params, "showGroups"); ok { + showGroups = v + } + if v, ok := paramBool(params, "showInvited"); ok { + showInvited = v + } + pageToken, _ := paramString(params, "pageToken") + + if pageSize <= 0 { + pageSize = maxResults + if pageSize <= 0 || pageSize > 100 { + pageSize = 100 + } + } + + ctx := context.Background() + _ = ctx // reserved for future use with .Pages() + + var aggregated *chat.ListMembershipsResponse + for { + call := svc.Spaces.Members.List(spaceName).PageSize(pageSize) + if filter != "" { + call = call.Filter(filter) + } + if showGroups { + call = call.ShowGroups(true) + } + if showInvited { + call = call.ShowInvited(true) + } + if pageToken != "" { + call = call.PageToken(pageToken) + } + + resp, err := call.Do() + if err != nil { + return fmt.Errorf("failed to list members: %w", err) + } + + if aggregated == nil { + aggregated = resp + } else { + aggregated.Memberships = append(aggregated.Memberships, resp.Memberships...) + aggregated.NextPageToken = resp.NextPageToken + } + + if resp.NextPageToken == "" { + break + } + if !fetchAll { + break + } + if maxResults > 0 && int64(len(aggregated.Memberships)) >= maxResults { + break + } + pageToken = resp.NextPageToken + } + + if aggregated == nil { + aggregated = &chat.ListMembershipsResponse{} + } + if maxResults > 0 && int64(len(aggregated.Memberships)) > maxResults { + aggregated.Memberships = aggregated.Memberships[:maxResults] + } + if fetchAll { + aggregated.NextPageToken = "" + } + return printRaw(aggregated) +} + +// chatRawServiceFromCmd is a small helper used by the existing runners to +// construct a chat client for the raw paths without changing their +// signatures. +func chatRawServiceFromCmd() (*chat.Service, error) { + ctx := context.Background() + factory, err := client.NewFactory(ctx) + if err != nil { + return nil, err + } + return factory.Chat() +} diff --git a/cmd/commands_test.go b/cmd/commands_test.go index c55640d..2fab697 100644 --- a/cmd/commands_test.go +++ b/cmd/commands_test.go @@ -550,6 +550,7 @@ func TestChatCommands(t *testing.T) { {"build-cache"}, {"find-group"}, {"find-space"}, + {"spaces"}, } for _, tt := range tests { @@ -560,6 +561,35 @@ func TestChatCommands(t *testing.T) { } }) } + + // Noun-verb API-shape paths added for --raw/--params support. + if findSubcommand(chatSpacesCmd, "list") == nil { + t.Fatal("expected 'chat spaces list'") + } + if findSubcommand(chatMessagesCmd, "list") == nil { + t.Fatal("expected 'chat messages list'") + } + if findSubcommand(chatMembersCmd, "list") == nil { + t.Fatal("expected 'chat members list'") + } +} + +// TestPeopleCommand verifies the gws people get command surface used by +// the programmatic --raw/--params path (#188). +func TestPeopleCommand(t *testing.T) { + if peopleCmd == nil { + t.Fatal("people command not registered") + } + get := findSubcommand(peopleCmd, "get") + if get == nil { + t.Fatal("people get not registered") + } + if get.Flags().Lookup("raw") == nil { + t.Error("people get missing --raw flag") + } + if get.Flags().Lookup("params") == nil { + t.Error("people get missing --params flag") + } } // TestFormsCommands tests forms command structure diff --git a/cmd/contacts.go b/cmd/contacts.go index b03c2d7..35252ef 100644 --- a/cmd/contacts.go +++ b/cmd/contacts.go @@ -301,14 +301,33 @@ func runContactsGet(cmd *cobra.Command, args []string) error { } resourceName := args[0] + pf := personFields - person, err := svc.People.Get(resourceName). - PersonFields(personFields). - Do() + params, perr := parseParams(cmd) + if perr != nil { + return p.PrintError(perr) + } + if v, ok := paramString(params, "resourceName"); ok && v != "" { + resourceName = v + } + if v, ok := paramString(params, "personFields"); ok && v != "" { + pf = v + } + + call := svc.People.Get(resourceName).PersonFields(pf) + if sources, ok := paramStringSlice(params, "sources"); ok && len(sources) > 0 { + call = call.Sources(sources...) + } + + person, err := call.Do() if err != nil { return p.PrintError(fmt.Errorf("failed to get contact: %w", err)) } + if isRaw(cmd) { + return printRaw(person) + } + return p.Print(formatPerson(person)) } diff --git a/cmd/gmail.go b/cmd/gmail.go index 0a3c5cc..78f1695 100644 --- a/cmd/gmail.go +++ b/cmd/gmail.go @@ -329,6 +329,8 @@ func init() { gmailListCmd.Flags().String("query", "", "Gmail search query (e.g., 'is:unread', 'from:someone@example.com')") gmailListCmd.Flags().Bool("all", false, "Fetch all matching results (may take time for large result sets)") gmailListCmd.Flags().Bool("include-labels", false, "Include Gmail label IDs in output") + addRawParamsFlags(gmailListCmd) + addRawParamsFlags(gmailThreadCmd) // Send flags gmailSendCmd.Flags().String("to", "", "Recipient email address (required)") @@ -475,6 +477,10 @@ func runGmailList(cmd *cobra.Command, args []string) error { fetchAll, _ := cmd.Flags().GetBool("all") includeLabels, _ := cmd.Flags().GetBool("include-labels") + if isRaw(cmd) { + return runGmailListRaw(cmd, svc, query, maxResults, fetchAll) + } + // Gmail API has a hard limit of 500 results per request const apiMaxPerPage int64 = 500 @@ -592,6 +598,110 @@ func runGmailList(cmd *cobra.Command, args []string) error { }) } +// runGmailListRaw implements `gmail list --raw`. It routes through +// users.messages.list (which is what the issue's raw shape documents) and +// emits the SDK response struct as JSON. --all aggregates the `messages` +// field across pages and drops nextPageToken; the final shape is +// {"messages":[...], "resultSizeEstimate":N}. +func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxResults int64, fetchAll bool) error { + params, perr := parseParams(cmd) + if perr != nil { + return perr + } + + // --params overrides flag-derived values (documented precedence: params win). + if v, ok := paramString(params, "q"); ok { + query = v + } + if v, ok := paramInt64(params, "maxResults"); ok { + maxResults = v + } + pageToken, _ := paramString(params, "pageToken") + includeSpamTrash, includeSpamTrashSet := paramBool(params, "includeSpamTrash") + labelIds, labelIdsSet := paramStringSlice(params, "labelIds") + + const apiMaxPerPage int64 = 500 + + var aggregated *gmail.ListMessagesResponse + pageNum := 1 + + for { + perPage := apiMaxPerPage + if !fetchAll && maxResults > 0 { + collected := int64(0) + if aggregated != nil { + collected = int64(len(aggregated.Messages)) + } + remaining := maxResults - collected + if remaining <= 0 { + break + } + if remaining < perPage { + perPage = remaining + } + } + + call := svc.Users.Messages.List("me").MaxResults(perPage) + if query != "" { + call = call.Q(query) + } + if pageToken != "" { + call = call.PageToken(pageToken) + } + if includeSpamTrashSet { + call = call.IncludeSpamTrash(includeSpamTrash) + } + if labelIdsSet && len(labelIds) > 0 { + call = call.LabelIds(labelIds...) + } + + resp, err := call.Do() + if err != nil { + return fmt.Errorf("failed to list messages: %w", err) + } + + if aggregated == nil { + aggregated = resp + } else { + aggregated.Messages = append(aggregated.Messages, resp.Messages...) + aggregated.NextPageToken = resp.NextPageToken + aggregated.ResultSizeEstimate = resp.ResultSizeEstimate + } + + if resp.NextPageToken != "" && (fetchAll || maxResults > apiMaxPerPage) { + fmt.Fprintf(os.Stderr, "Fetched page %d (%d messages so far)...\n", pageNum, len(aggregated.Messages)) + } + + if resp.NextPageToken == "" { + break + } + if !fetchAll && int64(len(aggregated.Messages)) >= maxResults { + break + } + + // In single-page mode (no --all, no --params pageToken), stop after one page. + if !fetchAll && maxResults <= apiMaxPerPage { + break + } + + pageToken = resp.NextPageToken + pageNum++ + } + + if aggregated == nil { + aggregated = &gmail.ListMessagesResponse{} + } + if !fetchAll && maxResults > 0 && int64(len(aggregated.Messages)) > maxResults { + aggregated.Messages = aggregated.Messages[:maxResults] + } + if fetchAll { + // Aggregated output: drop nextPageToken per the issue spec. + aggregated.NextPageToken = "" + } + + return printRaw(aggregated) +} + func runGmailRead(cmd *cobra.Command, args []string) error { p := GetPrinter() ctx := context.Background() @@ -1128,6 +1238,10 @@ func runGmailThread(cmd *cobra.Command, args []string) error { threadID := args[0] + if isRaw(cmd) { + return runGmailThreadRaw(cmd, svc, threadID) + } + thread, err := svc.Users.Threads.Get("me", threadID).Format("full").Do() if err != nil { return p.PrintError(fmt.Errorf("failed to get thread: %w", err)) @@ -1170,6 +1284,39 @@ func runGmailThread(cmd *cobra.Command, args []string) error { }) } +// runGmailThreadRaw implements `gmail thread --raw`. Emits the +// users.threads.get response as-is: `messages[*].payload.headers` as +// {name,value} arrays, `payload.parts` as a nested tree, base64url +// body data in `payload.body.data` / `parts[*].body.data`, plus +// `internalDate`, `labelIds`, `snippet`, etc. +func runGmailThreadRaw(cmd *cobra.Command, svc *gmail.Service, threadID string) error { + params, perr := parseParams(cmd) + if perr != nil { + return perr + } + + // --params overrides: id (resource path), format, metadataHeaders. + if v, ok := paramString(params, "id"); ok && v != "" { + threadID = v + } + format := "full" + if v, ok := paramString(params, "format"); ok && v != "" { + format = v + } + metadataHeaders, hasMetadataHeaders := paramStringSlice(params, "metadataHeaders") + + call := svc.Users.Threads.Get("me", threadID).Format(format) + if hasMetadataHeaders && len(metadataHeaders) > 0 { + call = call.MetadataHeaders(metadataHeaders...) + } + + thread, err := call.Do() + if err != nil { + return fmt.Errorf("failed to get thread: %w", err) + } + return printRaw(thread) +} + func runGmailEventID(cmd *cobra.Command, args []string) error { p := GetPrinter() ctx := context.Background() diff --git a/cmd/people.go b/cmd/people.go new file mode 100644 index 0000000..6076cbd --- /dev/null +++ b/cmd/people.go @@ -0,0 +1,104 @@ +package cmd + +// `gws people get` wraps People.Get for programmatic consumers. The default +// ergonomic People surface stays under `gws contacts` — this command exists +// because the People API JSON shape (resourceName, personFields-driven +// fields like emailAddresses[].metadata.source, etc.) is what scripts want +// when speaking the API directly. Backed by --raw + --params per #188. + +import ( + "context" + "errors" + "fmt" + + "github.com/omriariav/workspace-cli/internal/client" + "github.com/spf13/cobra" +) + +var peopleCmd = &cobra.Command{ + Use: "people", + Short: "Direct People API access", + Long: `Direct wrappers around the People API. Designed for programmatic +consumers — see also "gws contacts" for the ergonomic surface.`, +} + +var peopleGetCmd = &cobra.Command{ + Use: "get [resource-name]", + Short: "Get a person by resourceName", + Long: `Get a person by resourceName. + +Either pass as a positional argument or supply it via --params. +Required parameter: personFields (defaults to the legacy default for +"gws contacts get" when omitted). + +Examples: + gws people get --params '{"resourceName":"people/me","personFields":"emailAddresses"}' --raw + gws people get people/me --params '{"personFields":"names,emailAddresses"}'`, + Args: cobra.MaximumNArgs(1), + RunE: runPeopleGet, +} + +func init() { + rootCmd.AddCommand(peopleCmd) + peopleCmd.AddCommand(peopleGetCmd) + addRawParamsFlags(peopleGetCmd) + peopleGetCmd.Flags().String("person-fields", "", "Comma-separated personFields mask (overridden by --params personFields)") + + // Surface --raw + --params on the existing `gws contacts get` too so + // callers that already use that command can opt into the raw shape. + addRawParamsFlags(contactsGetCmd) +} + +func runPeopleGet(cmd *cobra.Command, args []string) error { + ctx := context.Background() + factory, err := client.NewFactory(ctx) + if err != nil { + return err + } + svc, err := factory.People() + if err != nil { + return err + } + + params, perr := parseParams(cmd) + if perr != nil { + return perr + } + + resourceName := "" + if len(args) > 0 { + resourceName = args[0] + } + if v, ok := paramString(params, "resourceName"); ok && v != "" { + resourceName = v + } + if resourceName == "" { + return errors.New("people get: resourceName is required (positional arg or --params resourceName)") + } + + pf, _ := cmd.Flags().GetString("person-fields") + if v, ok := paramString(params, "personFields"); ok && v != "" { + pf = v + } + if pf == "" { + pf = personFields + } + + call := svc.People.Get(resourceName).PersonFields(pf) + if sources, ok := paramStringSlice(params, "sources"); ok && len(sources) > 0 { + call = call.Sources(sources...) + } + if v, ok := paramString(params, "requestMask.includeField"); ok && v != "" { + call = call.RequestMaskIncludeField(v) + } + + person, err := call.Do() + if err != nil { + return fmt.Errorf("failed to get person: %w", err) + } + + if isRaw(cmd) { + return printRaw(person) + } + return GetPrinter().Print(formatPerson(person)) +} diff --git a/cmd/raw.go b/cmd/raw.go new file mode 100644 index 0000000..54fef12 --- /dev/null +++ b/cmd/raw.go @@ -0,0 +1,164 @@ +package cmd + +// Programmatic-mode plumbing for `--raw` and `--params`. +// +// `--raw` : emit the unmodified Google API response JSON (no field +// renaming, no body decoding, no header collapsing). Default +// ergonomic output is untouched when this flag is not set. +// `--params` : JSON object whose keys map directly to the underlying Google +// API request parameters. Keys supplied here override the +// equivalent CLI flags (params win), so callers can rely on the +// JSON payload being the source of truth. +// +// Pagination under `--raw`: +// * Without `--all` we emit the single page response verbatim. +// * With `--all` we concatenate the top-level list field across pages and +// drop `nextPageToken` from the final aggregated object. + +import ( + "encoding/json" + "fmt" + "io" + "os" + "strings" + + "github.com/spf13/cobra" +) + +// addRawParamsFlags registers `--raw` and `--params` on a leaf command. +func addRawParamsFlags(cmd *cobra.Command) { + cmd.Flags().Bool("raw", false, "Emit raw Google API response JSON (no transform). With --all, concatenates list fields and drops nextPageToken.") + cmd.Flags().String("params", "", "JSON object of raw API parameters. Keys override the equivalent CLI flags (params win).") +} + +// isRaw reports whether `--raw` is set on the command. +func isRaw(cmd *cobra.Command) bool { + if cmd == nil { + return false + } + if f := cmd.Flags().Lookup("raw"); f == nil { + return false + } + v, _ := cmd.Flags().GetBool("raw") + return v +} + +// parseParams returns the parsed `--params` payload, or nil when not set. +func parseParams(cmd *cobra.Command) (map[string]interface{}, error) { + if cmd == nil { + return nil, nil + } + if f := cmd.Flags().Lookup("params"); f == nil { + return nil, nil + } + raw, _ := cmd.Flags().GetString("params") + raw = strings.TrimSpace(raw) + if raw == "" { + return nil, nil + } + dec := json.NewDecoder(strings.NewReader(raw)) + dec.UseNumber() + var m map[string]interface{} + if err := dec.Decode(&m); err != nil { + return nil, fmt.Errorf("--params: invalid JSON: %w", err) + } + return m, nil +} + +// printRaw marshals v to stdout using SDK JSON tags directly so the output +// preserves the Google API shape. +func printRaw(v interface{}) error { + return writeRaw(os.Stdout, v) +} + +func writeRaw(w io.Writer, v interface{}) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(v) +} + +// Param accessors. All accept the parsed map (which may be nil) and a key, +// and return (value, ok). They tolerate the common JSON encodings for each +// type (numbers may decode as float64 or json.Number). + +func paramString(m map[string]interface{}, key string) (string, bool) { + if m == nil { + return "", false + } + v, ok := m[key] + if !ok || v == nil { + return "", false + } + s, ok := v.(string) + return s, ok +} + +func paramInt64(m map[string]interface{}, key string) (int64, bool) { + if m == nil { + return 0, false + } + v, ok := m[key] + if !ok || v == nil { + return 0, false + } + switch x := v.(type) { + case json.Number: + n, err := x.Int64() + if err != nil { + return 0, false + } + return n, true + case float64: + return int64(x), true + case int: + return int64(x), true + case int64: + return x, true + case string: + // Some callers pass numeric strings; accept them. + var n int64 + if _, err := fmt.Sscan(x, &n); err == nil { + return n, true + } + } + return 0, false +} + +func paramBool(m map[string]interface{}, key string) (bool, bool) { + if m == nil { + return false, false + } + v, ok := m[key] + if !ok || v == nil { + return false, false + } + b, ok := v.(bool) + return b, ok +} + +// paramStringSlice supports both ["a","b"] and "a,b" forms. +func paramStringSlice(m map[string]interface{}, key string) ([]string, bool) { + if m == nil { + return nil, false + } + v, ok := m[key] + if !ok || v == nil { + return nil, false + } + switch x := v.(type) { + case []interface{}: + out := make([]string, 0, len(x)) + for _, item := range x { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out, true + case string: + if x == "" { + return nil, true + } + return strings.Split(x, ","), true + } + return nil, false +} diff --git a/cmd/raw_test.go b/cmd/raw_test.go new file mode 100644 index 0000000..3ca4f6e --- /dev/null +++ b/cmd/raw_test.go @@ -0,0 +1,346 @@ +package cmd + +// Snapshot-style coverage for --raw output shape and --params parsing. +// +// We do not drive the Cobra runners end-to-end (those need real OAuth via +// client.NewFactory). Instead, we marshal the same SDK response structs the +// runners emit and assert that the JSON keys/structure match Google's +// public API reference for each endpoint. + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "strings" + "testing" + + "github.com/spf13/cobra" + chat "google.golang.org/api/chat/v1" + gmail "google.golang.org/api/gmail/v1" + people "google.golang.org/api/people/v1" +) + +// requireKeys decodes raw JSON and fails if any of want is missing. +func requireKeys(t *testing.T, raw []byte, want ...string) map[string]interface{} { + t.Helper() + var m map[string]interface{} + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + if err := dec.Decode(&m); err != nil { + t.Fatalf("decode: %v\nraw=%s", err, string(raw)) + } + for _, k := range want { + if _, ok := m[k]; !ok { + t.Errorf("expected top-level key %q in %s", k, string(raw)) + } + } + return m +} + +// --- parseParams / param accessors --------------------------------------- + +func newFlagCmd(rawJSON string, raw bool) *cobra.Command { + c := &cobra.Command{Use: "x"} + addRawParamsFlags(c) + if rawJSON != "" { + _ = c.Flags().Set("params", rawJSON) + } + if raw { + _ = c.Flags().Set("raw", "true") + } + return c +} + +func TestParseParams_EmptyAndMissing(t *testing.T) { + c := newFlagCmd("", false) + m, err := parseParams(c) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if m != nil { + t.Fatalf("expected nil map, got %v", m) + } +} + +func TestParseParams_InvalidJSON(t *testing.T) { + c := newFlagCmd("not json", false) + if _, err := parseParams(c); err == nil { + t.Fatal("expected error for invalid JSON") + } +} + +func TestParseParams_PreservesNumbersAndStrings(t *testing.T) { + c := newFlagCmd(`{"pageSize":50,"filter":"createTime > \"2025-01-01T00:00:00Z\"","showDeleted":true,"sources":["READ_SOURCE_TYPE_CONTACT"]}`, true) + m, err := parseParams(c) + if err != nil { + t.Fatalf("err: %v", err) + } + if n, ok := paramInt64(m, "pageSize"); !ok || n != 50 { + t.Errorf("pageSize: got (%d,%v)", n, ok) + } + if s, ok := paramString(m, "filter"); !ok || !strings.Contains(s, "createTime >") { + t.Errorf("filter: got (%q,%v)", s, ok) + } + if b, ok := paramBool(m, "showDeleted"); !ok || !b { + t.Errorf("showDeleted: got (%v,%v)", b, ok) + } + if ss, ok := paramStringSlice(m, "sources"); !ok || len(ss) != 1 || ss[0] != "READ_SOURCE_TYPE_CONTACT" { + t.Errorf("sources: got (%v,%v)", ss, ok) + } + if !isRaw(c) { + t.Error("expected --raw to be true") + } +} + +// --- API shape: gmail.users.messages.list -------------------------------- + +func TestRawShape_GmailListMessages(t *testing.T) { + resp := &gmail.ListMessagesResponse{ + Messages: []*gmail.Message{ + {Id: "18abc", ThreadId: "thr-1"}, + {Id: "18def", ThreadId: "thr-2"}, + }, + NextPageToken: "ZZZ", + ResultSizeEstimate: 42, + } + raw, err := json.Marshal(resp) + if err != nil { + t.Fatal(err) + } + m := requireKeys(t, raw, "messages", "nextPageToken", "resultSizeEstimate") + msgs, _ := m["messages"].([]interface{}) + if len(msgs) != 2 { + t.Fatalf("expected 2 messages, got %d", len(msgs)) + } + first, _ := msgs[0].(map[string]interface{}) + if _, ok := first["id"]; !ok { + t.Errorf("expected message.id key, got %v", first) + } + if _, ok := first["threadId"]; !ok { + t.Errorf("expected camelCase 'threadId', got %v", first) + } +} + +// --- API shape: gmail.users.threads.get ---------------------------------- + +func TestRawShape_GmailThreadGet(t *testing.T) { + body := base64.URLEncoding.EncodeToString([]byte("hello world")) + thread := &gmail.Thread{ + Id: "thread-1", + HistoryId: 1234, + Messages: []*gmail.Message{ + { + Id: "msg-1", + ThreadId: "thread-1", + LabelIds: []string{"INBOX", "UNREAD"}, + Snippet: "hello", + InternalDate: 1700000000000, + Payload: &gmail.MessagePart{ + MimeType: "multipart/alternative", + Headers: []*gmail.MessagePartHeader{ + {Name: "Subject", Value: "Hi"}, + {Name: "From", Value: "a@b.com"}, + }, + Parts: []*gmail.MessagePart{ + { + MimeType: "text/plain", + Body: &gmail.MessagePartBody{Data: body, Size: 11}, + }, + }, + }, + }, + }, + } + raw, err := json.Marshal(thread) + if err != nil { + t.Fatal(err) + } + + // Top-level shape: id, messages[], historyId. + requireKeys(t, raw, "id", "messages", "historyId") + + // Drill into messages[0].payload.headers — must be {name,value} array. + var threadOut struct { + Messages []struct { + Id string `json:"id"` + LabelIds []string `json:"labelIds"` + Snippet string `json:"snippet"` + InternalDate string `json:"internalDate"` + Payload struct { + MimeType string `json:"mimeType"` + Headers []struct { + Name string `json:"name"` + Value string `json:"value"` + } `json:"headers"` + Parts []struct { + MimeType string `json:"mimeType"` + Body struct { + Data string `json:"data"` + Size int64 `json:"size"` + } `json:"body"` + } `json:"parts"` + } `json:"payload"` + } `json:"messages"` + } + if err := json.Unmarshal(raw, &threadOut); err != nil { + t.Fatalf("decode: %v", err) + } + if len(threadOut.Messages) != 1 { + t.Fatalf("expected 1 message, got %d", len(threadOut.Messages)) + } + msg := threadOut.Messages[0] + if msg.InternalDate == "" { + t.Error("expected internalDate (Gmail API returns it as a string of millis)") + } + if len(msg.LabelIds) != 2 { + t.Errorf("expected 2 labelIds, got %v", msg.LabelIds) + } + if len(msg.Payload.Headers) != 2 || msg.Payload.Headers[0].Name != "Subject" { + t.Errorf("expected headers as {name,value} array, got %+v", msg.Payload.Headers) + } + if len(msg.Payload.Parts) != 1 || msg.Payload.Parts[0].Body.Data != body { + t.Errorf("expected nested parts[*].body.data preserved (base64), got %+v", msg.Payload.Parts) + } +} + +// --- API shape: chat.spaces.list ----------------------------------------- + +func TestRawShape_ChatListSpaces(t *testing.T) { + resp := &chat.ListSpacesResponse{ + Spaces: []*chat.Space{ + {Name: "spaces/AAA", DisplayName: "Team", SpaceType: "SPACE"}, + {Name: "spaces/BBB", DisplayName: "Direct", SpaceType: "DIRECT_MESSAGE"}, + }, + NextPageToken: "tok", + } + raw, err := json.Marshal(resp) + if err != nil { + t.Fatal(err) + } + m := requireKeys(t, raw, "spaces", "nextPageToken") + spaces, _ := m["spaces"].([]interface{}) + if len(spaces) != 2 { + t.Fatalf("expected 2 spaces, got %d", len(spaces)) + } + first, _ := spaces[0].(map[string]interface{}) + for _, k := range []string{"name", "displayName", "spaceType"} { + if _, ok := first[k]; !ok { + t.Errorf("expected space.%s key, got %v", k, first) + } + } +} + +// --- API shape: chat.spaces.members.list --------------------------------- + +func TestRawShape_ChatListMemberships(t *testing.T) { + resp := &chat.ListMembershipsResponse{ + Memberships: []*chat.Membership{ + {Name: "spaces/AAA/members/m1", Role: "ROLE_MEMBER", Member: &chat.User{Name: "users/123", Type: "HUMAN", DisplayName: "Alice"}}, + }, + NextPageToken: "tok", + } + raw, err := json.Marshal(resp) + if err != nil { + t.Fatal(err) + } + m := requireKeys(t, raw, "memberships", "nextPageToken") + members, _ := m["memberships"].([]interface{}) + if len(members) != 1 { + t.Fatalf("expected 1 membership, got %d", len(members)) + } + memb, _ := members[0].(map[string]interface{}) + for _, k := range []string{"name", "role", "member"} { + if _, ok := memb[k]; !ok { + t.Errorf("expected membership.%s key", k) + } + } + user, _ := memb["member"].(map[string]interface{}) + for _, k := range []string{"name", "type", "displayName"} { + if _, ok := user[k]; !ok { + t.Errorf("expected member.%s key, got %v", k, user) + } + } +} + +// --- API shape: chat.spaces.messages.list -------------------------------- + +func TestRawShape_ChatListMessages(t *testing.T) { + resp := &chat.ListMessagesResponse{ + Messages: []*chat.Message{ + { + Name: "spaces/AAA/messages/msg-1", + Sender: &chat.User{Name: "users/123", DisplayName: "Alice"}, + CreateTime: "2026-04-01T10:00:00Z", + Text: "hello", + Thread: &chat.Thread{Name: "spaces/AAA/threads/t1"}, + }, + }, + NextPageToken: "tok", + } + raw, err := json.Marshal(resp) + if err != nil { + t.Fatal(err) + } + m := requireKeys(t, raw, "messages", "nextPageToken") + msgs, _ := m["messages"].([]interface{}) + first, _ := msgs[0].(map[string]interface{}) + for _, k := range []string{"name", "sender", "createTime", "text", "thread"} { + if _, ok := first[k]; !ok { + t.Errorf("expected message.%s key, got %v", k, first) + } + } + sender, _ := first["sender"].(map[string]interface{}) + if _, ok := sender["displayName"]; !ok { + t.Errorf("expected sender.displayName camelCase, got %v", sender) + } +} + +// --- API shape: people.people.get ---------------------------------------- + +func TestRawShape_PeopleGet(t *testing.T) { + person := &people.Person{ + ResourceName: "people/c12345", + Etag: "etag-x", + Names: []*people.Name{ + {DisplayName: "Alice", GivenName: "Alice"}, + }, + EmailAddresses: []*people.EmailAddress{ + {Value: "alice@example.com", Type: "work"}, + }, + } + raw, err := json.Marshal(person) + if err != nil { + t.Fatal(err) + } + m := requireKeys(t, raw, "resourceName", "etag", "names", "emailAddresses") + names, _ := m["names"].([]interface{}) + if len(names) != 1 { + t.Fatalf("expected 1 name, got %d", len(names)) + } + first, _ := names[0].(map[string]interface{}) + for _, k := range []string{"displayName", "givenName"} { + if _, ok := first[k]; !ok { + t.Errorf("expected name.%s key, got %v", k, first) + } + } + emails, _ := m["emailAddresses"].([]interface{}) + firstEmail, _ := emails[0].(map[string]interface{}) + for _, k := range []string{"value", "type"} { + if _, ok := firstEmail[k]; !ok { + t.Errorf("expected emailAddress.%s key, got %v", k, firstEmail) + } + } +} + +// --- writeRaw produces stable indented JSON ------------------------------ + +func TestWriteRaw_IndentedJSON(t *testing.T) { + var buf bytes.Buffer + if err := writeRaw(&buf, map[string]interface{}{"k": "v"}); err != nil { + t.Fatal(err) + } + out := buf.String() + if !strings.Contains(out, "\n \"k\": \"v\"") { + t.Errorf("expected 2-space indented JSON, got %q", out) + } +} From 900423e88d10747796054f9a92a7857181b9460d Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 22:25:06 +0300 Subject: [PATCH 02/12] fix(chat_raw): drop unused chatRawServiceFromCmd helper Lint flagged it as unused (CI's golangci-lint fail). The runners pull the chat service through the existing factory in chat.go before branching into the raw paths, so the helper was dead code. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/chat_raw.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/cmd/chat_raw.go b/cmd/chat_raw.go index 79cff0b..182c877 100644 --- a/cmd/chat_raw.go +++ b/cmd/chat_raw.go @@ -10,11 +10,9 @@ package cmd // is concatenated across pages and nextPageToken is dropped. import ( - "context" "errors" "fmt" - "github.com/omriariav/workspace-cli/internal/client" "github.com/spf13/cobra" "google.golang.org/api/chat/v1" ) @@ -294,9 +292,6 @@ func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, } } - ctx := context.Background() - _ = ctx // reserved for future use with .Pages() - var aggregated *chat.ListMembershipsResponse for { call := svc.Spaces.Members.List(spaceName).PageSize(pageSize) @@ -348,15 +343,3 @@ func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, } return printRaw(aggregated) } - -// chatRawServiceFromCmd is a small helper used by the existing runners to -// construct a chat client for the raw paths without changing their -// signatures. -func chatRawServiceFromCmd() (*chat.Service, error) { - ctx := context.Background() - factory, err := client.NewFactory(ctx) - if err != nil { - return nil, err - } - return factory.Chat() -} From 99aaaadc50deaa81b0073d2180c63a80a1ca0b7f Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 22:35:05 +0300 Subject: [PATCH 03/12] fix: address codex review on PR #189 Codex review findings: 1. Critical: --all in raw chat runners was silently capped by --max default (25 for messages, 100 for members). When --all is set we now ignore --max (0 = unlimited). Same guard added to chat spaces raw for symmetry. 2. Warning: printRaw bypassed the global --quiet contract. It now short-circuits when quiet is set, matching GetPrinter's NullPrinter. 3. Warning: --params maxResults was not mapped directly to the Gmail API parameter under --all (always sent the API's 500 max). It now maps directly: --params maxResults is the per-page parameter; --max stays the total-results cap. --all drops --max for symmetry with the chat fix. 4. Warning: tests didn't exercise the Cobra runners. Added cmd/raw_runners_test.go covering httptest-driven runs: * gmail list raw: --all aggregates, drops nextPageToken, --params overrides flag-derived query/page size * gmail list raw: --quiet suppresses output * gmail thread raw: --params format passes through * chat spaces list raw: --all aggregates, --params overrides * chat messages list raw: --all ignores 25-default cap (regression test for the critical bug above) * chat members list raw: --all ignores 100-default cap * people get raw: resourceName/personFields passthrough 5. Warning: skill references didn't mention the new commands/flags. Updated skills/gmail/references/commands.md, skills/chat/..., skills/contacts/... with the new flags, the spaces|messages| members list aliases, and the new `gws people get`. 6. README command tables now list --raw/--params alongside the relevant rows and a new "People" section. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 21 +- cmd/chat_raw.go | 19 ++ cmd/gmail.go | 44 ++- cmd/raw.go | 7 +- cmd/raw_runners_test.go | 437 +++++++++++++++++++++++++ skills/chat/references/commands.md | 40 ++- skills/contacts/references/commands.md | 29 +- skills/gmail/references/commands.md | 12 +- 8 files changed, 586 insertions(+), 23 deletions(-) create mode 100644 cmd/raw_runners_test.go diff --git a/README.md b/README.md index 77e3182..4a7d0e2 100644 --- a/README.md +++ b/README.md @@ -94,9 +94,9 @@ Add `--format text` for human-readable output, or `--format yaml` for YAML. | Command | Description | |---------|-------------| -| `gws gmail list` | List threads with `thread_id` and `message_id` (`--max`, `--query`, `--all`, `--include-labels`) | +| `gws gmail list` | List threads with `thread_id` and `message_id` (`--max`, `--query`, `--all`, `--include-labels`, `--raw`, `--params`). Under `--raw` switches to `users.messages.list` shape. | | `gws gmail read ` | Read message body and headers | -| `gws gmail thread ` | Read full thread conversation | +| `gws gmail thread ` | Read full thread conversation (`--raw`, `--params`) | | `gws gmail send` | Send email (`--to`, `--subject`, `--body`, `--cc`, `--bcc`, `--thread-id`, `--reply-to-message-id`, `--attachment`) | | `gws gmail reply ` | Reply to message (`--body`, `--cc`, `--bcc`, `--all`) | | `gws gmail forward ` | Forward message (`--to`, `--body`, `--cc`, `--bcc`) | @@ -347,10 +347,13 @@ Add `--format text` for human-readable output, or `--format yaml` for YAML. | Command | Description | |---------|-------------| -| `gws chat list` | List spaces (`--filter`, `--page-size`) | +| `gws chat list` | List spaces (`--filter`, `--page-size`, `--raw`, `--params`) | +| `gws chat spaces list` | List spaces (API-shape friendly path; same as `chat list` with `--raw` / `--params` documented examples) | | `gws chat recent` | Recap messages across active spaces (`--since`, `--max`, `--max-per-space`, `--max-spaces`) | -| `gws chat messages ` | List messages (`--max`, `--filter`, `--order-by`, `--show-deleted`, `--after`, `--before`, `--resolve-senders`) | -| `gws chat members ` | List members with display names + emails via People API (`--max`, `--filter`, `--show-groups`, `--show-invited`) | +| `gws chat messages ` | List messages (`--max`, `--filter`, `--order-by`, `--show-deleted`, `--after`, `--before`, `--resolve-senders`, `--raw`, `--params`) | +| `gws chat messages list` | List messages by `parent` via `--params` (programmatic path) | +| `gws chat members ` | List members with display names + emails via People API (`--max`, `--filter`, `--show-groups`, `--show-invited`, `--raw`, `--params`) | +| `gws chat members list` | List members by `parent` via `--params` (programmatic path) | | `gws chat send` | Send message (`--space`, `--text`) | | `gws chat get ` | Get a single message (`--resolve-senders`) | | `gws chat update ` | Update message text (`--text`) | @@ -397,10 +400,16 @@ Add `--format text` for human-readable output, or `--format yaml` for YAML. |---------|-------------| | `gws contacts list` | List contacts (`--max`) | | `gws contacts search ` | Search contacts by name/email/phone | -| `gws contacts get ` | Get contact details | +| `gws contacts get ` | Get contact details (`--raw`, `--params`) | | `gws contacts create` | Create contact (`--name`, `--email`, `--phone`) | | `gws contacts delete ` | Delete a contact | +### People (programmatic People API) + +| Command | Description | +|---------|-------------| +| `gws people get [resource-name]` | Direct People.Get wrapper for programmatic use (`--raw`, `--params`, `--person-fields`). Use `--params '{"resourceName":"people/me","personFields":"emailAddresses"}'`. | + ### Groups > Requires Admin SDK API enabled and Workspace admin privileges. diff --git a/cmd/chat_raw.go b/cmd/chat_raw.go index 182c877..9d621b2 100644 --- a/cmd/chat_raw.go +++ b/cmd/chat_raw.go @@ -114,6 +114,12 @@ func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSi } pageToken, _ := paramString(params, "pageToken") + // --all means "fetch every page". --max (0 already means unlimited + // for spaces, but guard for symmetry with the other raw runners). + if fetchAll { + maxResults = 0 + } + if pageSize <= 0 { pageSize = 100 } @@ -197,6 +203,13 @@ func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, } pageToken, _ := paramString(params, "pageToken") + // --all means "fetch every page". The CLI's --max default (25) must + // not silently cap an --all run; callers who want a cap should drop + // --all and pass --max explicitly. + if fetchAll { + maxResults = 0 + } + var aggregated *chat.ListMessagesResponse for { thisPage := pageSize @@ -285,6 +298,12 @@ func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, } pageToken, _ := paramString(params, "pageToken") + // --all means "fetch every page". The CLI's --max default (100) must + // not silently cap an --all run. + if fetchAll { + maxResults = 0 + } + if pageSize <= 0 { pageSize = maxResults if pageSize <= 0 || pageSize > 100 { diff --git a/cmd/gmail.go b/cmd/gmail.go index 78f1695..f7e27b7 100644 --- a/cmd/gmail.go +++ b/cmd/gmail.go @@ -609,25 +609,45 @@ func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxRe return perr } - // --params overrides flag-derived values (documented precedence: params win). + // --params overrides flag-derived values (documented precedence: + // params win). `maxResults` in --params maps directly to the Google + // API's per-page parameter (this is what the API reference calls + // "maxResults"). The CLI `--max` flag is a separate total-results + // cap applied to the aggregated response. if v, ok := paramString(params, "q"); ok { query = v } - if v, ok := paramInt64(params, "maxResults"); ok { - maxResults = v - } pageToken, _ := paramString(params, "pageToken") includeSpamTrash, includeSpamTrashSet := paramBool(params, "includeSpamTrash") labelIds, labelIdsSet := paramStringSlice(params, "labelIds") const apiMaxPerPage int64 = 500 + paramPageSize, paramPageSizeSet := paramInt64(params, "maxResults") + + // --all means "fetch every page": ignore --max (a 10-default would + // silently cap an --all run otherwise). + if fetchAll { + maxResults = 0 + } var aggregated *gmail.ListMessagesResponse pageNum := 1 for { - perPage := apiMaxPerPage - if !fetchAll && maxResults > 0 { + // Determine page size for this request: --params maxResults + // wins if set, otherwise size by remaining budget capped at the + // API max. + var perPage int64 + switch { + case paramPageSizeSet && paramPageSize > 0: + perPage = paramPageSize + if perPage > apiMaxPerPage { + perPage = apiMaxPerPage + } + default: + perPage = apiMaxPerPage + } + if maxResults > 0 { collected := int64(0) if aggregated != nil { collected = int64(len(aggregated.Messages)) @@ -636,7 +656,7 @@ func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxRe if remaining <= 0 { break } - if remaining < perPage { + if !paramPageSizeSet && remaining < perPage { perPage = remaining } } @@ -675,12 +695,12 @@ func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxRe if resp.NextPageToken == "" { break } - if !fetchAll && int64(len(aggregated.Messages)) >= maxResults { + if maxResults > 0 && int64(len(aggregated.Messages)) >= maxResults { break } - - // In single-page mode (no --all, no --params pageToken), stop after one page. - if !fetchAll && maxResults <= apiMaxPerPage { + // Single-page mode: stop after the first page unless --all asks + // us to keep paging. + if !fetchAll { break } @@ -691,7 +711,7 @@ func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxRe if aggregated == nil { aggregated = &gmail.ListMessagesResponse{} } - if !fetchAll && maxResults > 0 && int64(len(aggregated.Messages)) > maxResults { + if maxResults > 0 && int64(len(aggregated.Messages)) > maxResults { aggregated.Messages = aggregated.Messages[:maxResults] } if fetchAll { diff --git a/cmd/raw.go b/cmd/raw.go index 54fef12..a31a6d0 100644 --- a/cmd/raw.go +++ b/cmd/raw.go @@ -66,8 +66,13 @@ func parseParams(cmd *cobra.Command) (map[string]interface{}, error) { } // printRaw marshals v to stdout using SDK JSON tags directly so the output -// preserves the Google API shape. +// preserves the Google API shape. Honors the global --quiet flag: when +// quiet is set, output is suppressed (matching GetPrinter's NullPrinter +// contract so scripts can run raw commands quietly for side effects). func printRaw(v interface{}) error { + if quiet { + return nil + } return writeRaw(os.Stdout, v) } diff --git a/cmd/raw_runners_test.go b/cmd/raw_runners_test.go new file mode 100644 index 0000000..9251332 --- /dev/null +++ b/cmd/raw_runners_test.go @@ -0,0 +1,437 @@ +package cmd + +// End-to-end coverage for the --raw / --params runners using httptest as +// the upstream. Verifies outgoing query params, --params-over-flags +// precedence, --all pagination merge (concat list + drop nextPageToken), +// and the --quiet contract for raw output. + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "sync" + "testing" + + "github.com/spf13/cobra" + chat "google.golang.org/api/chat/v1" + gmail "google.golang.org/api/gmail/v1" + "google.golang.org/api/option" + people "google.golang.org/api/people/v1" +) + +// captureStdout swaps os.Stdout for a pipe, runs fn, and returns the +// captured bytes. The runners write through printRaw → os.Stdout so this +// is how we observe them. +func captureStdout(t *testing.T, fn func() error) (string, error) { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stdout = w + + var buf bytes.Buffer + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + _, _ = io.Copy(&buf, r) + }() + + runErr := fn() + _ = w.Close() + wg.Wait() + os.Stdout = orig + return buf.String(), runErr +} + +// makeCmd builds a *cobra.Command with the given flag values applied — +// enough surface for the raw runners to read from. +func makeCmd(t *testing.T, flags map[string]string) *cobra.Command { + t.Helper() + c := &cobra.Command{Use: "x"} + addRawParamsFlags(c) + for k, v := range flags { + if err := c.Flags().Set(k, v); err != nil { + t.Fatalf("set %s=%s: %v", k, v, err) + } + } + return c +} + +// --- gmail list ---------------------------------------------------------- + +func TestGmailListRaw_AllAggregatesAndDropsToken(t *testing.T) { + var capturedQueries []url.Values + var mu sync.Mutex + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + capturedQueries = append(capturedQueries, r.URL.Query()) + mu.Unlock() + + w.Header().Set("Content-Type", "application/json") + page := r.URL.Query().Get("pageToken") + switch page { + case "": + _ = json.NewEncoder(w).Encode(&gmail.ListMessagesResponse{ + Messages: []*gmail.Message{{Id: "1", ThreadId: "t1"}, {Id: "2", ThreadId: "t2"}}, + NextPageToken: "p2", + ResultSizeEstimate: 4, + }) + case "p2": + _ = json.NewEncoder(w).Encode(&gmail.ListMessagesResponse{ + Messages: []*gmail.Message{{Id: "3", ThreadId: "t3"}, {Id: "4", ThreadId: "t4"}}, + ResultSizeEstimate: 4, + }) + default: + t.Fatalf("unexpected pageToken %q", page) + } + })) + defer server.Close() + + svc, err := gmail.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatalf("gmail service: %v", err) + } + + cmd := makeCmd(t, map[string]string{ + "raw": "true", + "params": `{"maxResults":2,"q":"in:sent"}`, + }) + + out, err := captureStdout(t, func() error { + return runGmailListRaw(cmd, svc, "ignored-by-params", 0, true) // fetchAll=true + }) + if err != nil { + t.Fatalf("runner err: %v", err) + } + + // Decode and verify aggregated shape. + var got gmail.ListMessagesResponse + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("decode: %v\nout=%s", err, out) + } + if len(got.Messages) != 4 { + t.Errorf("expected 4 aggregated messages, got %d (%s)", len(got.Messages), out) + } + if got.NextPageToken != "" { + t.Errorf("expected nextPageToken dropped under --all, got %q", got.NextPageToken) + } + + // --params q must have overridden the flag-derived "ignored-by-params". + if len(capturedQueries) < 1 { + t.Fatal("expected at least one request") + } + if q := capturedQueries[0].Get("q"); q != "in:sent" { + t.Errorf("expected q=in:sent from --params, got %q", q) + } + if ms := capturedQueries[0].Get("maxResults"); ms != "2" { + t.Errorf("expected maxResults=2 from --params (per-page), got %q", ms) + } +} + +func TestGmailListRaw_QuietSuppressesOutput(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(&gmail.ListMessagesResponse{ + Messages: []*gmail.Message{{Id: "1", ThreadId: "t1"}}, + }) + })) + defer server.Close() + + svc, err := gmail.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatal(err) + } + + prev := quiet + quiet = true + defer func() { quiet = prev }() + + cmd := makeCmd(t, map[string]string{"raw": "true"}) + out, err := captureStdout(t, func() error { + return runGmailListRaw(cmd, svc, "", 10, false) + }) + if err != nil { + t.Fatalf("runner err: %v", err) + } + if out != "" { + t.Errorf("expected no output under --quiet, got %q", out) + } +} + +// --- gmail thread -------------------------------------------------------- + +func TestGmailThreadRaw_PreservesAPIShape(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.Path, "/threads/thr-x") { + t.Fatalf("unexpected path %s", r.URL.Path) + } + if got := r.URL.Query().Get("format"); got != "metadata" { + t.Errorf("expected format=metadata from --params, got %q", got) + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(&gmail.Thread{ + Id: "thr-x", + Messages: []*gmail.Message{ + { + Id: "m1", + LabelIds: []string{"INBOX"}, + Payload: &gmail.MessagePart{ + Headers: []*gmail.MessagePartHeader{ + {Name: "Subject", Value: "Hi"}, + }, + }, + }, + }, + }) + })) + defer server.Close() + + svc, err := gmail.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatal(err) + } + + cmd := makeCmd(t, map[string]string{ + "raw": "true", + "params": `{"format":"metadata"}`, + }) + out, err := captureStdout(t, func() error { + return runGmailThreadRaw(cmd, svc, "thr-x") + }) + if err != nil { + t.Fatalf("runner err: %v", err) + } + + var m map[string]interface{} + if err := json.Unmarshal([]byte(out), &m); err != nil { + t.Fatalf("decode: %v\nout=%s", err, out) + } + for _, k := range []string{"id", "messages"} { + if _, ok := m[k]; !ok { + t.Errorf("expected key %q, got %v", k, m) + } + } +} + +// --- chat spaces list ---------------------------------------------------- + +func TestChatListRaw_AllAggregatesAndParamsOverride(t *testing.T) { + var qs []url.Values + var mu sync.Mutex + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + qs = append(qs, r.URL.Query()) + mu.Unlock() + + w.Header().Set("Content-Type", "application/json") + token := r.URL.Query().Get("pageToken") + switch token { + case "": + _ = json.NewEncoder(w).Encode(&chat.ListSpacesResponse{ + Spaces: []*chat.Space{{Name: "spaces/A"}, {Name: "spaces/B"}}, + NextPageToken: "next-1", + }) + case "next-1": + _ = json.NewEncoder(w).Encode(&chat.ListSpacesResponse{ + Spaces: []*chat.Space{{Name: "spaces/C"}}, + }) + default: + t.Fatalf("unexpected token %q", token) + } + })) + defer server.Close() + + svc, err := chat.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatal(err) + } + + cmd := makeCmd(t, map[string]string{ + "raw": "true", + "params": `{"pageSize":7,"filter":"spaceType = \"DIRECT_MESSAGE\""}`, + }) + out, err := captureStdout(t, func() error { + // flag-derived filter is "OTHER" — --params must override. + return runChatListRaw(cmd, svc, "OTHER", 100, 0, true) + }) + if err != nil { + t.Fatalf("runner err: %v", err) + } + + var got chat.ListSpacesResponse + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("decode: %v\nout=%s", err, out) + } + if len(got.Spaces) != 3 { + t.Errorf("expected 3 aggregated spaces, got %d", len(got.Spaces)) + } + if got.NextPageToken != "" { + t.Errorf("expected nextPageToken dropped under --all, got %q", got.NextPageToken) + } + if len(qs) < 1 { + t.Fatal("expected requests") + } + if f := qs[0].Get("filter"); !strings.Contains(f, "DIRECT_MESSAGE") { + t.Errorf("expected filter from --params, got %q", f) + } + if p := qs[0].Get("pageSize"); p != "7" { + t.Errorf("expected pageSize=7 from --params, got %q", p) + } +} + +// --- chat messages list (the --all bug from codex review) ---------------- + +func TestChatMessagesRaw_AllIgnoresDefaultMaxCap(t *testing.T) { + // Server returns 30 messages across two pages — more than the default + // --max of 25 from chatMessagesListCmd. With --all, the runner must + // not cap the result. + page1 := make([]*chat.Message, 25) + for i := range page1 { + page1[i] = &chat.Message{Name: "spaces/AAA/messages/p1-" + string(rune('a'+i))} + } + page2 := []*chat.Message{ + {Name: "spaces/AAA/messages/p2-a"}, + {Name: "spaces/AAA/messages/p2-b"}, + {Name: "spaces/AAA/messages/p2-c"}, + {Name: "spaces/AAA/messages/p2-d"}, + {Name: "spaces/AAA/messages/p2-e"}, + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + token := r.URL.Query().Get("pageToken") + if token == "" { + _ = json.NewEncoder(w).Encode(&chat.ListMessagesResponse{Messages: page1, NextPageToken: "p2"}) + } else { + _ = json.NewEncoder(w).Encode(&chat.ListMessagesResponse{Messages: page2}) + } + })) + defer server.Close() + + svc, err := chat.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatal(err) + } + + cmd := makeCmd(t, map[string]string{"raw": "true"}) + // maxResults=25 simulates the default flag value; fetchAll=true. + out, err := captureStdout(t, func() error { + return runChatMessagesRaw(cmd, svc, "spaces/AAA", 25, "", "", false, true) + }) + if err != nil { + t.Fatalf("runner err: %v", err) + } + + var got chat.ListMessagesResponse + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("decode: %v\nout=%s", err, out) + } + if len(got.Messages) != 30 { + t.Errorf("expected --all to return all 30 messages, got %d", len(got.Messages)) + } + if got.NextPageToken != "" { + t.Errorf("expected nextPageToken dropped, got %q", got.NextPageToken) + } +} + +// --- chat members list --------------------------------------------------- + +func TestChatMembersRaw_AllIgnoresDefaultMaxCap(t *testing.T) { + // 150 members across two pages; default --max=100 must not cap --all. + page1 := make([]*chat.Membership, 100) + for i := range page1 { + page1[i] = &chat.Membership{Name: "spaces/AAA/members/p1-" + string(rune('a'+(i%26)))} + } + page2 := make([]*chat.Membership, 50) + for i := range page2 { + page2[i] = &chat.Membership{Name: "spaces/AAA/members/p2-" + string(rune('a'+(i%26)))} + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + token := r.URL.Query().Get("pageToken") + if token == "" { + _ = json.NewEncoder(w).Encode(&chat.ListMembershipsResponse{Memberships: page1, NextPageToken: "p2"}) + } else { + _ = json.NewEncoder(w).Encode(&chat.ListMembershipsResponse{Memberships: page2}) + } + })) + defer server.Close() + + svc, err := chat.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatal(err) + } + + cmd := makeCmd(t, map[string]string{"raw": "true"}) + out, err := captureStdout(t, func() error { + return runChatMembersRaw(cmd, svc, "spaces/AAA", 100, "", false, false, true) + }) + if err != nil { + t.Fatalf("runner err: %v", err) + } + var got chat.ListMembershipsResponse + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("decode: %v\nout=%s", err, out) + } + if len(got.Memberships) != 150 { + t.Errorf("expected --all to return all 150 memberships, got %d", len(got.Memberships)) + } +} + +// --- people get ---------------------------------------------------------- + +func TestPeopleGetRaw_HonorsParamsResourceNameAndPersonFields(t *testing.T) { + var path string + var query url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path = r.URL.Path + query = r.URL.Query() + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(&people.Person{ + ResourceName: "people/me", + Etag: "etag-1", + EmailAddresses: []*people.EmailAddress{ + {Value: "me@example.com"}, + }, + }) + })) + defer server.Close() + + svc, err := people.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatal(err) + } + + // Drive through the People.Get call to verify the API parameter + // shape (this mirrors what runPeopleGet builds via params). + person, err := svc.People.Get("people/me").PersonFields("emailAddresses").Do() + if err != nil { + t.Fatalf("people get: %v", err) + } + if person.ResourceName != "people/me" { + t.Errorf("expected resourceName=people/me, got %q", person.ResourceName) + } + if !strings.HasSuffix(path, "/people/me") { + t.Errorf("expected path to end in /people/me, got %q", path) + } + if got := query.Get("personFields"); got != "emailAddresses" { + t.Errorf("expected personFields=emailAddresses, got %q", got) + } + // Sanity-check the raw shape via printRaw. + out, err := captureStdout(t, func() error { return printRaw(person) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, `"resourceName"`) || !strings.Contains(out, `"emailAddresses"`) { + t.Errorf("expected raw People shape, got %s", out) + } +} diff --git a/skills/chat/references/commands.md b/skills/chat/references/commands.md index 0d1903e..7f1d52c 100644 --- a/skills/chat/references/commands.md +++ b/skills/chat/references/commands.md @@ -34,14 +34,30 @@ Usage: gws chat list [flags] |------|------|---------|-------------| | `--filter` | string | | Filter spaces (e.g. `spaceType = "SPACE"`) | | `--page-size` | int | 100 | Number of spaces per page | +| `--all` | bool | false | Fetch every page (raw mode aggregates the list field and drops `nextPageToken`) | +| `--raw` | bool | false | Emit unmodified `spaces.list` response JSON | +| `--params` | string | | JSON object mapped to `spaces.list` request parameters (`pageSize`, `filter`, `pageToken`). Overrides equivalent CLI flags. | ### Output Fields (JSON) -Each space includes: +Default ergonomic output. Each space includes: - `name` — Space resource name (e.g., `spaces/AAAA1234`) - `displayName` — Human-readable space name - `type` — Space type (`ROOM`, `DM`, `GROUP_CHAT`) +Under `--raw` the response shape matches Google's `spaces.list` reference exactly (`{"spaces":[...],"nextPageToken":"..."}`). + +--- + +## gws chat spaces list + +Programmatic alias for `gws chat list` that mirrors the API method name. Same flag surface; useful when scripting against `spaces.list`. + +``` +Usage: gws chat spaces list [flags] +gws chat spaces list --params '{"pageSize":50,"filter":"spaceType = \"DIRECT_MESSAGE\""}' --raw --all +``` + --- ## gws chat messages @@ -61,6 +77,17 @@ Usage: gws chat messages [flags] | `--order-by` | string | | Order messages (e.g. `createTime DESC`) | | `--show-deleted` | bool | false | Include deleted messages in results | | `--resolve-senders` | bool | false | Make extra API calls to fill missing `sender_display_name` (via space membership listing) and add `self` (via People API `people/me`). | +| `--all` | bool | false | Fetch every page (raw mode aggregates the `messages` field; ignores `--max`) | +| `--raw` | bool | false | Emit unmodified `spaces.messages.list` response JSON | +| `--params` | string | | JSON object mapped to `spaces.messages.list` request parameters (`parent`, `pageSize`, `filter`, `orderBy`, `showDeleted`, `pageToken`). Overrides equivalent CLI flags. | + +### gws chat messages list + +Programmatic alias that accepts `parent` via `--params` instead of a positional argument: + +``` +gws chat messages list --params '{"parent":"spaces/AAA","pageSize":50,"filter":"createTime > \"2025-01-01T00:00:00Z\""}' --raw --all +``` `--after` and `--before` are convenience flags that translate to filter expressions. They combine with `--filter` using AND. @@ -88,6 +115,17 @@ Usage: gws chat members [flags] | `--filter` | string | | Filter members (e.g. `member.type = "HUMAN"`) | | `--show-groups` | bool | false | Include Google Group memberships | | `--show-invited` | bool | false | Include invited memberships | +| `--all` | bool | false | Fetch every page (raw mode aggregates the `memberships` field; ignores `--max`) | +| `--raw` | bool | false | Emit unmodified `spaces.members.list` response JSON | +| `--params` | string | | JSON object mapped to `spaces.members.list` request parameters (`parent`, `pageSize`, `filter`, `showGroups`, `showInvited`, `pageToken`). Overrides equivalent CLI flags. | + +### gws chat members list + +Programmatic alias that accepts `parent` via `--params` instead of a positional argument: + +``` +gws chat members list --params '{"parent":"spaces/AAA","pageSize":50}' --raw --all +``` --- diff --git a/skills/contacts/references/commands.md b/skills/contacts/references/commands.md index 79c8dc5..407fe36 100644 --- a/skills/contacts/references/commands.md +++ b/skills/contacts/references/commands.md @@ -138,17 +138,42 @@ Usage: gws contacts get |----------|------|----------|-------------| | `resource-name` | string | Yes | Resource identifier (e.g., `people/c1234567890`) | -No additional flags. +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--raw` | bool | false | Emit unmodified `people.get` response JSON | +| `--params` | string | | JSON object mapped to `people.get` request parameters (`resourceName`, `personFields`, `sources`). Overrides equivalent CLI flags. | ### Output Fields (JSON) -Returns a single contact object with: +Default ergonomic output is a single contact object with: - `resource_name` — Resource identifier - `name` — Contact's display name - `emails` — Array of email addresses (if available) - `phones` — Array of phone numbers (if available) - `organization` — Object with `name` and `title` (if available) +Under `--raw` the response shape matches Google's `people.get` reference exactly (`resourceName`, `etag`, `names[]`, `emailAddresses[]`, etc.). + +--- + +## gws people get + +Programmatic People.Get wrapper. Designed for scripting; pass `resourceName` and `personFields` directly as the API expects. + +``` +Usage: gws people get [resource-name] [flags] +``` + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--person-fields` | string | (legacy default) | Comma-separated personFields mask (overridden by `--params personFields`) | +| `--raw` | bool | false | Emit unmodified `people.get` response JSON | +| `--params` | string | | JSON object mapped to `people.get` request parameters (`resourceName`, `personFields`, `sources`, `requestMask.includeField`). Overrides equivalent CLI flags. | + +``` +gws people get --params '{"resourceName":"people/me","personFields":"emailAddresses"}' --raw +``` + ### Resource Name Format Resource names follow the pattern: `people/c` diff --git a/skills/gmail/references/commands.md b/skills/gmail/references/commands.md index 5cf24ff..dec721e 100644 --- a/skills/gmail/references/commands.md +++ b/skills/gmail/references/commands.md @@ -30,6 +30,8 @@ Usage: gws gmail list [flags] | `--all` | bool | false | Fetch all matching results (may take time for large result sets) | | `--query` | string | | Gmail search query | | `--include-labels` | bool | false | Include Gmail label IDs in output | +| `--raw` | bool | false | Emit unmodified `users.messages.list` response JSON (switches the underlying call from `threads.list` to `messages.list`) | +| `--params` | string | | JSON object mapped to `users.messages.list` request parameters (`q`, `maxResults` [per-page], `pageToken`, `includeSpamTrash`, `labelIds`). Overrides equivalent CLI flags. | ### Output Fields (JSON) @@ -97,10 +99,16 @@ Reads and displays all messages in a Gmail thread (conversation). Usage: gws gmail thread ``` -No additional flags. Use the `thread_id` from `gws gmail list` output. +Use the `thread_id` from `gws gmail list` output. + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--raw` | bool | false | Emit unmodified `users.threads.get` response JSON (full payload tree, base64 `parts[*].body.data`, `headers` as `{name,value}` arrays, `internalDate`, `labelIds`, `snippet`) | +| `--params` | string | | JSON object mapped to `users.threads.get` request parameters (`id`, `format`, `metadataHeaders`). Overrides equivalent CLI flags. | ### Output Fields (JSON) +Default ergonomic output: - `thread_id` — Thread ID - `message_count` — Number of messages in thread - `messages` — Array of messages, each with: @@ -110,6 +118,8 @@ No additional flags. Use the `thread_id` from `gws gmail list` output. - `labels` — Applied label IDs - `attachments` — Array (omitted when empty); same shape as `gws gmail read` +Under `--raw` the response shape matches Google's `users.threads.get` reference exactly. + --- ## gws gmail send From 450d410c05c1b4521caf30e8e9d76e1b1286413b Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 22:41:30 +0300 Subject: [PATCH 04/12] fix: address codex round-2 review on PR #189 1. Raw mode is verbatim by default. The CLI's --max flag would previously slice a verbatim API response (e.g. --params maxResults=50 with default --max=10 trimmed to 10). Each raw runner now only honors --max when the caller explicitly set it (cmd.Flags().Changed("max")); the default is no cap. --all still disables --max for "fetch every page" semantics. Affected: runGmailListRaw, runChatListRaw, runChatMessagesRaw, runChatMembersRaw (and their httptest cases updated). 2. chat messages --after / --before were silently dropped under --raw because the raw dispatch happened before they were folded into the filter expression. Moved the filter combination above the raw dispatch so both code paths see the same query. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/chat.go | 22 ++++++++++++---------- cmd/chat_raw.go | 27 +++++++++++++++------------ cmd/gmail.go | 14 ++++++++++++-- cmd/raw_runners_test.go | 10 +++++----- 4 files changed, 44 insertions(+), 29 deletions(-) diff --git a/cmd/chat.go b/cmd/chat.go index 8fef735..12b295a 100644 --- a/cmd/chat.go +++ b/cmd/chat.go @@ -587,7 +587,7 @@ func runChatList(cmd *cobra.Command, args []string) error { } if isRaw(cmd) { - return runChatListRaw(cmd, svc, filter, pageSize, maxResults, fetchAll) + return runChatListRaw(cmd, svc, filter, pageSize, maxResults, fetchAll, cmd.Flags().Changed("max")) } var results []map[string]interface{} @@ -660,14 +660,9 @@ func runChatMessages(cmd *cobra.Command, args []string) error { fetchAll, _ = cmd.Flags().GetBool("all") } - if isRaw(cmd) { - return runChatMessagesRaw(cmd, svc, spaceName, maxResults, filter, orderBy, showDeleted, fetchAll) - } - if spaceName == "" { - return p.PrintError(errors.New("chat messages: a space id is required")) - } - - // Build filter from --after/--before flags, combining with --filter + // Fold --after/--before into the filter expression up-front so the + // raw path sees the same query the ergonomic path does. (Before this + // move, --after/--before were silently dropped under --raw.) var filterParts []string if after != "" { filterParts = append(filterParts, fmt.Sprintf(`createTime > "%s"`, after)) @@ -682,6 +677,13 @@ func runChatMessages(cmd *cobra.Command, args []string) error { filter = strings.Join(filterParts, " AND ") } + if isRaw(cmd) { + return runChatMessagesRaw(cmd, svc, spaceName, maxResults, filter, orderBy, showDeleted, fetchAll, cmd.Flags().Changed("max")) + } + if spaceName == "" { + return p.PrintError(errors.New("chat messages: a space id is required")) + } + if maxResults <= 0 { return p.Print(map[string]interface{}{ "messages": []map[string]interface{}{}, @@ -1063,7 +1065,7 @@ func runChatMembers(cmd *cobra.Command, args []string) error { } if isRaw(cmd) { - return runChatMembersRaw(cmd, svc, spaceName, maxResults, filter, showGroups, showInvited, fetchAll) + return runChatMembersRaw(cmd, svc, spaceName, maxResults, filter, showGroups, showInvited, fetchAll, cmd.Flags().Changed("max")) } if spaceName == "" { return p.PrintError(errors.New("chat members: a space id is required")) diff --git a/cmd/chat_raw.go b/cmd/chat_raw.go index 9d621b2..039f8ab 100644 --- a/cmd/chat_raw.go +++ b/cmd/chat_raw.go @@ -100,7 +100,7 @@ func init() { } // runChatListRaw implements `gws chat spaces list --raw` (and `gws chat list --raw`). -func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSize, maxResults int64, fetchAll bool) error { +func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSize, maxResults int64, fetchAll, maxExplicit bool) error { params, perr := parseParams(cmd) if perr != nil { return perr @@ -114,8 +114,12 @@ func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSi } pageToken, _ := paramString(params, "pageToken") - // --all means "fetch every page". --max (0 already means unlimited - // for spaces, but guard for symmetry with the other raw runners). + // Raw mode is verbatim: only honor --max when the caller set it. + if !maxExplicit { + maxResults = 0 + } + // --all means "fetch every page" — drop --max even if it was set + // (already 0 from above for the default case). if fetchAll { maxResults = 0 } @@ -176,7 +180,7 @@ func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSi } // runChatMessagesRaw implements `gws chat messages list --raw`. -func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter, orderBy string, showDeleted, fetchAll bool) error { +func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter, orderBy string, showDeleted, fetchAll, maxExplicit bool) error { params, perr := parseParams(cmd) if perr != nil { return perr @@ -203,10 +207,9 @@ func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, } pageToken, _ := paramString(params, "pageToken") - // --all means "fetch every page". The CLI's --max default (25) must - // not silently cap an --all run; callers who want a cap should drop - // --all and pass --max explicitly. - if fetchAll { + // Raw mode is verbatim: drop the CLI default --max unless the + // caller explicitly set it. --all also disables --max for symmetry. + if !maxExplicit || fetchAll { maxResults = 0 } @@ -271,7 +274,7 @@ func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, } // runChatMembersRaw implements `gws chat members list --raw`. -func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter string, showGroups, showInvited, fetchAll bool) error { +func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter string, showGroups, showInvited, fetchAll, maxExplicit bool) error { params, perr := parseParams(cmd) if perr != nil { return perr @@ -298,9 +301,9 @@ func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, } pageToken, _ := paramString(params, "pageToken") - // --all means "fetch every page". The CLI's --max default (100) must - // not silently cap an --all run. - if fetchAll { + // Raw mode is verbatim: drop the CLI default --max unless the + // caller explicitly set it. --all also disables --max. + if !maxExplicit || fetchAll { maxResults = 0 } diff --git a/cmd/gmail.go b/cmd/gmail.go index f7e27b7..53fc881 100644 --- a/cmd/gmail.go +++ b/cmd/gmail.go @@ -478,7 +478,11 @@ func runGmailList(cmd *cobra.Command, args []string) error { includeLabels, _ := cmd.Flags().GetBool("include-labels") if isRaw(cmd) { - return runGmailListRaw(cmd, svc, query, maxResults, fetchAll) + // In raw mode `--max` only caps the response when the user + // explicitly set it; otherwise we leave the API response + // verbatim (the --raw contract is "no modifications"). + maxExplicit := cmd.Flags().Changed("max") + return runGmailListRaw(cmd, svc, query, maxResults, fetchAll, maxExplicit) } // Gmail API has a hard limit of 500 results per request @@ -603,11 +607,17 @@ func runGmailList(cmd *cobra.Command, args []string) error { // emits the SDK response struct as JSON. --all aggregates the `messages` // field across pages and drops nextPageToken; the final shape is // {"messages":[...], "resultSizeEstimate":N}. -func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxResults int64, fetchAll bool) error { +func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxResults int64, fetchAll, maxExplicit bool) error { params, perr := parseParams(cmd) if perr != nil { return perr } + // Raw mode preserves the API response verbatim. The CLI's --max + // default would otherwise slice a verbatim page, breaking the + // --raw contract. Only honor --max when the caller set it. + if !maxExplicit { + maxResults = 0 + } // --params overrides flag-derived values (documented precedence: // params win). `maxResults` in --params maps directly to the Google diff --git a/cmd/raw_runners_test.go b/cmd/raw_runners_test.go index 9251332..171c169 100644 --- a/cmd/raw_runners_test.go +++ b/cmd/raw_runners_test.go @@ -108,7 +108,7 @@ func TestGmailListRaw_AllAggregatesAndDropsToken(t *testing.T) { }) out, err := captureStdout(t, func() error { - return runGmailListRaw(cmd, svc, "ignored-by-params", 0, true) // fetchAll=true + return runGmailListRaw(cmd, svc, "ignored-by-params", 0, true, false) // fetchAll=true }) if err != nil { t.Fatalf("runner err: %v", err) @@ -158,7 +158,7 @@ func TestGmailListRaw_QuietSuppressesOutput(t *testing.T) { cmd := makeCmd(t, map[string]string{"raw": "true"}) out, err := captureStdout(t, func() error { - return runGmailListRaw(cmd, svc, "", 10, false) + return runGmailListRaw(cmd, svc, "", 10, false, true) }) if err != nil { t.Fatalf("runner err: %v", err) @@ -262,7 +262,7 @@ func TestChatListRaw_AllAggregatesAndParamsOverride(t *testing.T) { }) out, err := captureStdout(t, func() error { // flag-derived filter is "OTHER" — --params must override. - return runChatListRaw(cmd, svc, "OTHER", 100, 0, true) + return runChatListRaw(cmd, svc, "OTHER", 100, 0, true, false) }) if err != nil { t.Fatalf("runner err: %v", err) @@ -325,7 +325,7 @@ func TestChatMessagesRaw_AllIgnoresDefaultMaxCap(t *testing.T) { cmd := makeCmd(t, map[string]string{"raw": "true"}) // maxResults=25 simulates the default flag value; fetchAll=true. out, err := captureStdout(t, func() error { - return runChatMessagesRaw(cmd, svc, "spaces/AAA", 25, "", "", false, true) + return runChatMessagesRaw(cmd, svc, "spaces/AAA", 25, "", "", false, true, false) }) if err != nil { t.Fatalf("runner err: %v", err) @@ -373,7 +373,7 @@ func TestChatMembersRaw_AllIgnoresDefaultMaxCap(t *testing.T) { cmd := makeCmd(t, map[string]string{"raw": "true"}) out, err := captureStdout(t, func() error { - return runChatMembersRaw(cmd, svc, "spaces/AAA", 100, "", false, false, true) + return runChatMembersRaw(cmd, svc, "spaces/AAA", 100, "", false, false, true, false) }) if err != nil { t.Fatalf("runner err: %v", err) From 1ffe7539452633da383825d9c8e621817a395d99 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 22:46:31 +0300 Subject: [PATCH 05/12] fix: address codex round-3 review on PR #189 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. runPeopleGet now has a real httptest runner test. Refactored runPeopleGet into a thin wrapper around runPeopleGetWithSvc (takes *people.Service) so we can drive it against an httptest backend without OAuth. New tests cover: - --params resourceName/personFields override positional args and CLI flags - --person-fields flag is the fallback when --params has no personFields - missing resourceName produces an explicit error The previous "test" only exercised svc.People.Get directly, not the runner — codex was right to call that out. 2. parseParams now rejects trailing junk after the JSON object. Previously '{"pageSize":50} garbage' silently parsed the first object; now it errors with "unexpected trailing data". Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/people.go | 7 +++ cmd/raw.go | 5 ++ cmd/raw_runners_test.go | 114 ++++++++++++++++++++++++++++++++-------- 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/cmd/people.go b/cmd/people.go index 6076cbd..1063f83 100644 --- a/cmd/people.go +++ b/cmd/people.go @@ -13,6 +13,7 @@ import ( "github.com/omriariav/workspace-cli/internal/client" "github.com/spf13/cobra" + people "google.golang.org/api/people/v1" ) var peopleCmd = &cobra.Command{ @@ -59,7 +60,13 @@ func runPeopleGet(cmd *cobra.Command, args []string) error { if err != nil { return err } + return runPeopleGetWithSvc(cmd, svc, args) +} +// runPeopleGetWithSvc is the testable inner half of runPeopleGet — takes +// an injected *people.Service so the runner can be exercised against an +// httptest backend without needing real OAuth. +func runPeopleGetWithSvc(cmd *cobra.Command, svc *people.Service, args []string) error { params, perr := parseParams(cmd) if perr != nil { return perr diff --git a/cmd/raw.go b/cmd/raw.go index a31a6d0..5df07c7 100644 --- a/cmd/raw.go +++ b/cmd/raw.go @@ -62,6 +62,11 @@ func parseParams(cmd *cobra.Command) (map[string]interface{}, error) { if err := dec.Decode(&m); err != nil { return nil, fmt.Errorf("--params: invalid JSON: %w", err) } + // Reject trailing junk after the object so scripts don't silently + // drop a typo'd second value. + if dec.More() { + return nil, fmt.Errorf("--params: unexpected trailing data after JSON object") + } return m, nil } diff --git a/cmd/raw_runners_test.go b/cmd/raw_runners_test.go index 171c169..dc8eb53 100644 --- a/cmd/raw_runners_test.go +++ b/cmd/raw_runners_test.go @@ -389,49 +389,117 @@ func TestChatMembersRaw_AllIgnoresDefaultMaxCap(t *testing.T) { // --- people get ---------------------------------------------------------- -func TestPeopleGetRaw_HonorsParamsResourceNameAndPersonFields(t *testing.T) { - var path string - var query url.Values +// peopleGetTestServer is shared scaffolding for the runner tests. +func peopleGetTestServer(t *testing.T, capture func(path string, q url.Values)) (*people.Service, func()) { + t.Helper() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - path = r.URL.Path - query = r.URL.Query() + if capture != nil { + capture(r.URL.Path, r.URL.Query()) + } w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(&people.Person{ ResourceName: "people/me", Etag: "etag-1", EmailAddresses: []*people.EmailAddress{ - {Value: "me@example.com"}, + {Value: "me@example.com", Type: "work"}, }, + Names: []*people.Name{{DisplayName: "Test User"}}, }) })) - defer server.Close() - svc, err := people.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) if err != nil { + server.Close() t.Fatal(err) } + return svc, server.Close +} + +func newPeopleGetCmd(t *testing.T, flags map[string]string) *cobra.Command { + t.Helper() + c := &cobra.Command{Use: "get"} + addRawParamsFlags(c) + c.Flags().String("person-fields", "", "") + for k, v := range flags { + if err := c.Flags().Set(k, v); err != nil { + t.Fatalf("set %s=%s: %v", k, v, err) + } + } + return c +} + +func TestRunPeopleGet_ParamsOverrideArgsAndSendsPersonFields(t *testing.T) { + var gotPath string + var gotQuery url.Values + svc, cleanup := peopleGetTestServer(t, func(p string, q url.Values) { + gotPath = p + gotQuery = q + }) + defer cleanup() - // Drive through the People.Get call to verify the API parameter - // shape (this mirrors what runPeopleGet builds via params). - person, err := svc.People.Get("people/me").PersonFields("emailAddresses").Do() + cmd := newPeopleGetCmd(t, map[string]string{ + "raw": "true", + "params": `{"resourceName":"people/me","personFields":"emailAddresses"}`, + }) + out, err := captureStdout(t, func() error { + // Positional arg is wrong on purpose — --params must win. + return runPeopleGetWithSvc(cmd, svc, []string{"people/ignored"}) + }) if err != nil { - t.Fatalf("people get: %v", err) + t.Fatalf("runner err: %v", err) } - if person.ResourceName != "people/me" { - t.Errorf("expected resourceName=people/me, got %q", person.ResourceName) + if !strings.HasSuffix(gotPath, "/people/me") { + t.Errorf("expected /people/me from --params, got %q", gotPath) } - if !strings.HasSuffix(path, "/people/me") { - t.Errorf("expected path to end in /people/me, got %q", path) + if pf := gotQuery.Get("personFields"); pf != "emailAddresses" { + t.Errorf("expected personFields=emailAddresses, got %q", pf) } - if got := query.Get("personFields"); got != "emailAddresses" { - t.Errorf("expected personFields=emailAddresses, got %q", got) + if !strings.Contains(out, `"resourceName"`) || !strings.Contains(out, `"emailAddresses"`) { + t.Errorf("expected raw People shape, got %s", out) + } + // Default ergonomic shape should not appear under --raw. + if strings.Contains(out, `"emails"`) { + t.Errorf("raw mode should not emit formatted 'emails', got %s", out) } - // Sanity-check the raw shape via printRaw. - out, err := captureStdout(t, func() error { return printRaw(person) }) +} + +func TestRunPeopleGet_PersonFieldsFlagFallback(t *testing.T) { + var gotQuery url.Values + svc, cleanup := peopleGetTestServer(t, func(_ string, q url.Values) { + gotQuery = q + }) + defer cleanup() + + cmd := newPeopleGetCmd(t, map[string]string{ + "person-fields": "names,emailAddresses", + }) + _, err := captureStdout(t, func() error { + return runPeopleGetWithSvc(cmd, svc, []string{"people/me"}) + }) if err != nil { - t.Fatal(err) + t.Fatalf("runner err: %v", err) } - if !strings.Contains(out, `"resourceName"`) || !strings.Contains(out, `"emailAddresses"`) { - t.Errorf("expected raw People shape, got %s", out) + if pf := gotQuery.Get("personFields"); pf != "names,emailAddresses" { + t.Errorf("expected personFields from --person-fields flag, got %q", pf) + } +} + +func TestRunPeopleGet_MissingResourceNameErrors(t *testing.T) { + svc, cleanup := peopleGetTestServer(t, nil) + defer cleanup() + + cmd := newPeopleGetCmd(t, nil) + err := runPeopleGetWithSvc(cmd, svc, nil) + if err == nil { + t.Fatal("expected error when resourceName missing") + } + if !strings.Contains(err.Error(), "resourceName is required") { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestParseParams_RejectsTrailingJunk(t *testing.T) { + c := newFlagCmd(`{"pageSize":50} garbage`, false) + if _, err := parseParams(c); err == nil { + t.Fatal("expected error for trailing junk after JSON") } } From 6c8e842c6e411b235179ec99ed1ceff479608ac2 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 22:52:35 +0300 Subject: [PATCH 06/12] fix: address codex round-4 review on PR #189 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. --all on existing chat messages/members commands was exposed but ignored in ergonomic mode. The new noun-verb list commands honor --all under --raw; the ergonomic commands intentionally don't. Updated help text so the flag's scope is unambiguous: chat messages --all → "Raw mode only: ..." chat members --all → "Raw mode only: ..." chat list --all → notes that --max=0 already returns all in ergonomic mode 2. contacts get docs mentioned `--params resourceName` but the command still had ExactArgs(1), so the positional was mandatory regardless. Relaxed to `[resource-name]` / MaximumNArgs(1) so the documented `--params resourceName` actually replaces the positional. Updated the existing TestContactsCommands assertion and the contacts skill reference to match. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/chat_raw.go | 11 ++++++++--- cmd/contacts.go | 18 +++++++++++++----- cmd/contacts_test.go | 2 +- skills/contacts/references/commands.md | 4 ++-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/cmd/chat_raw.go b/cmd/chat_raw.go index 039f8ab..3ecf53e 100644 --- a/cmd/chat_raw.go +++ b/cmd/chat_raw.go @@ -94,9 +94,14 @@ func init() { addRawParamsFlags(chatListCmd) addRawParamsFlags(chatMessagesCmd) addRawParamsFlags(chatMembersCmd) - chatListCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") - chatMessagesCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") - chatMembersCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field)") + // --all is wired here for symmetry with the new list commands, but + // the ergonomic chat messages/members runners do not honor it + // (their default ergonomic shape is per-space and intentionally + // bounded). Help text reflects that it only takes effect under + // --raw on these commands. + chatListCmd.Flags().Bool("all", false, "Fetch all matching results across pages (raw mode aggregates list field; ergonomic mode already returns all when --max=0)") + chatMessagesCmd.Flags().Bool("all", false, "Raw mode only: fetch all matching pages and concatenate (no effect without --raw)") + chatMembersCmd.Flags().Bool("all", false, "Raw mode only: fetch all matching pages and concatenate (no effect without --raw)") } // runChatListRaw implements `gws chat spaces list --raw` (and `gws chat list --raw`). diff --git a/cmd/contacts.go b/cmd/contacts.go index 35252ef..bf5246f 100644 --- a/cmd/contacts.go +++ b/cmd/contacts.go @@ -35,11 +35,13 @@ var contactsSearchCmd = &cobra.Command{ } var contactsGetCmd = &cobra.Command{ - Use: "get ", + Use: "get [resource-name]", Short: "Get contact details", - Long: "Gets detailed information about a specific contact by resource name (e.g., people/c1234567890).", - Args: cobra.ExactArgs(1), - RunE: runContactsGet, + Long: `Gets detailed information about a specific contact by resource name (e.g., people/c1234567890). + +Either pass as a positional argument or set "resourceName" via --params.`, + Args: cobra.MaximumNArgs(1), + RunE: runContactsGet, } var contactsCreateCmd = &cobra.Command{ @@ -300,7 +302,10 @@ func runContactsGet(cmd *cobra.Command, args []string) error { return p.PrintError(err) } - resourceName := args[0] + resourceName := "" + if len(args) > 0 { + resourceName = args[0] + } pf := personFields params, perr := parseParams(cmd) @@ -313,6 +318,9 @@ func runContactsGet(cmd *cobra.Command, args []string) error { if v, ok := paramString(params, "personFields"); ok && v != "" { pf = v } + if resourceName == "" { + return p.PrintError(fmt.Errorf("contacts get: resource-name is required (positional arg or --params resourceName)")) + } call := svc.People.Get(resourceName).PersonFields(pf) if sources, ok := paramStringSlice(params, "sources"); ok && len(sources) > 0 { diff --git a/cmd/contacts_test.go b/cmd/contacts_test.go index d311979..b698b25 100644 --- a/cmd/contacts_test.go +++ b/cmd/contacts_test.go @@ -19,7 +19,7 @@ func TestContactsCommands(t *testing.T) { }{ {"list", "list"}, {"search", "search "}, - {"get", "get "}, + {"get", "get [resource-name]"}, {"create", "create"}, {"delete", "delete "}, {"update", "update "}, diff --git a/skills/contacts/references/commands.md b/skills/contacts/references/commands.md index 407fe36..3778978 100644 --- a/skills/contacts/references/commands.md +++ b/skills/contacts/references/commands.md @@ -131,12 +131,12 @@ gws contacts search "Jane Smith" --format json | jq -r '.contacts[0].resource_na Gets detailed information about a specific contact by resource name. ``` -Usage: gws contacts get +Usage: gws contacts get [resource-name] [flags] ``` | Argument | Type | Required | Description | |----------|------|----------|-------------| -| `resource-name` | string | Yes | Resource identifier (e.g., `people/c1234567890`) | +| `resource-name` | string | One of | Resource identifier (e.g., `people/c1234567890`). Required unless supplied via `--params resourceName`. | | Flag | Type | Default | Description | |------|------|---------|-------------| From dc06b43b47e05286db920e6901aabe663e40e594 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 22:59:50 +0300 Subject: [PATCH 07/12] fix: address codex round-5 review on PR #189 1. gmail thread relaxed to [thread-id]: the docs/runner advertised --params id as an override, but ExactArgs(1) required the positional regardless. Now MaximumNArgs(1); runGmailThread passes through to runGmailThreadRaw under --raw, and the raw runner errors with an explicit message if neither positional nor --params id is set. Matches the contacts get / people get pattern from round 4. Tests + skill ref updated. 2. Version bumped to 1.39.0 per CLAUDE.md "Implementation Patterns" convention: feature PRs adding new commands bump VERSION. Also updated the "Current Version" line in CLAUDE.md with the v1.39 release summary describing --raw/--params and the new noun-verb paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 +- Makefile | 2 +- cmd/commands_test.go | 2 +- cmd/gmail.go | 22 ++++++++++++++++++---- cmd/gmail_test.go | 2 +- skills/gmail/references/commands.md | 2 +- 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5ea441a..f92d19e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,7 +50,7 @@ go run ./cmd/gws # or go run . ## Current Version -**v1.38.0** - Drive `resolve-comment` / `unresolve-comment` now use `Replies.Create` with `action=resolve|reopen` (preserves the original comment; optional `--content` for a closing/reopening note). Gmail `read` and `thread` expose an `attachments` array with `filename`, `mime_type`, `size`, `attachment_id`, and `part_id` so callers can chain into `gws gmail attachment`. New `gws chat recent --since <2h|7d|RFC3339>` recaps messages across active spaces using `lastActiveTime` as a prefilter, with `--max`, `--max-per-space`, `--max-spaces`, `--resolve-senders`, and `--exclude-self`. +**v1.39.0** - Adds `--raw` and `--params` to six list/get commands for programmatic, API-shape JSON output (skips field renaming, base64 decoding, header collapsing). New noun-verb command paths: `gws chat spaces list`, `gws chat messages list`, `gws chat members list`, and `gws people get`. `gmail list` switches to `users.messages.list` shape under `--raw`. `--all` aggregates the top-level list field across pages and drops `nextPageToken`. `--params` is a JSON object whose keys map directly to Google API request parameters and override the equivalent CLI flags (params win). `contacts get` and `gmail thread` relaxed to accept the resource id via `--params resourceName`/`--params id`. ## Roadmap diff --git a/Makefile b/Makefile index 94d9f41..1e6f71c 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ PKG := ./cmd/gws BUILD_DIR := ./bin # Version info -VERSION ?= 1.38.0 +VERSION ?= 1.39.0 COMMIT := $(shell git rev-parse --short HEAD 2>/dev/null || echo "unknown") BUILD_DATE := $(shell date -u +"%Y-%m-%dT%H:%M:%SZ") LDFLAGS := -ldflags "-X github.com/omriariav/workspace-cli/cmd.Version=$(VERSION) \ diff --git a/cmd/commands_test.go b/cmd/commands_test.go index 2fab697..8c6281e 100644 --- a/cmd/commands_test.go +++ b/cmd/commands_test.go @@ -124,7 +124,7 @@ func TestGmailCommands(t *testing.T) { {"archive", "archive ", true}, {"trash", "trash ", true}, {"archive-thread", "archive-thread ", true}, - {"thread", "thread ", true}, + {"thread", "thread [thread-id]", true}, {"event-id", "event-id ", true}, {"reply", "reply ", true}, {"forward", "forward ", true}, diff --git a/cmd/gmail.go b/cmd/gmail.go index 53fc881..3cd07a9 100644 --- a/cmd/gmail.go +++ b/cmd/gmail.go @@ -158,15 +158,18 @@ Examples: } var gmailThreadCmd = &cobra.Command{ - Use: "thread ", + Use: "thread [thread-id]", Short: "Read a full thread", Long: `Reads and displays all messages in a Gmail thread (conversation). Use the thread_id from "gws gmail list" to view the full conversation. +The thread id is required; pass it as a positional argument or supply it +via --params (e.g. '{"id":"18abc"}') when using --raw. Examples: - gws gmail thread 18abc123`, - Args: cobra.ExactArgs(1), + gws gmail thread 18abc123 + gws gmail thread --raw --params '{"id":"18abc123","format":"metadata"}'`, + Args: cobra.MaximumNArgs(1), RunE: runGmailThread, } @@ -1266,11 +1269,19 @@ func runGmailThread(cmd *cobra.Command, args []string) error { return p.PrintError(err) } - threadID := args[0] + threadID := "" + if len(args) > 0 { + threadID = args[0] + } if isRaw(cmd) { + // runGmailThreadRaw applies --params id; final emptiness is + // validated inside the raw runner. return runGmailThreadRaw(cmd, svc, threadID) } + if threadID == "" { + return p.PrintError(fmt.Errorf("gmail thread: a thread id is required")) + } thread, err := svc.Users.Threads.Get("me", threadID).Format("full").Do() if err != nil { @@ -1329,6 +1340,9 @@ func runGmailThreadRaw(cmd *cobra.Command, svc *gmail.Service, threadID string) if v, ok := paramString(params, "id"); ok && v != "" { threadID = v } + if threadID == "" { + return fmt.Errorf("gmail thread: a thread id is required (positional arg or --params id)") + } format := "full" if v, ok := paramString(params, "format"); ok && v != "" { format = v diff --git a/cmd/gmail_test.go b/cmd/gmail_test.go index c9a520e..2e36a85 100644 --- a/cmd/gmail_test.go +++ b/cmd/gmail_test.go @@ -1277,7 +1277,7 @@ func TestEmailMatchesSelf(t *testing.T) { func TestGmailThreadCommand_Help(t *testing.T) { cmd := gmailThreadCmd - if cmd.Use != "thread " { + if cmd.Use != "thread [thread-id]" { t.Errorf("unexpected Use: %s", cmd.Use) } diff --git a/skills/gmail/references/commands.md b/skills/gmail/references/commands.md index dec721e..80c5ec8 100644 --- a/skills/gmail/references/commands.md +++ b/skills/gmail/references/commands.md @@ -96,7 +96,7 @@ No additional flags. Use the `message_id` from `gws gmail list` output. Reads and displays all messages in a Gmail thread (conversation). ``` -Usage: gws gmail thread +Usage: gws gmail thread [thread-id] ``` Use the `thread_id` from `gws gmail list` output. From 35ac019839e9caee9efa1ed1b133b84c84150833 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 23:06:36 +0300 Subject: [PATCH 08/12] fix: address codex round-6 review on PR #189 1. Chat raw runners clamp page size to remaining --max budget. The previous code requested the default pageSize (100), then sliced the response. That left nextPageToken pointing past item 100 even though we returned items 1-N, so a client continuing pagination would skip the items we sliced off. Now: when --max is set and !fetchAll, pageSize is clamped to (remaining-budget). Verbatim nextPageToken now stays aligned with the returned slice. New TestChatListRaw_ClampsPageSizeToMax test covers the bug. 2. `people` is now a recognized auth service alias for `contacts`. The new `gws people get` command needs the same People API scopes as `gws contacts`. Users can now run `gws auth login --services people` and scoped-auth warnings can refer to either name interchangeably. ServiceForScope iterates a deterministic order so `contacts` remains canonical when a scope maps to both aliases. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/chat_raw.go | 26 +++++++++++++++++++++++ cmd/raw_runners_test.go | 47 +++++++++++++++++++++++++++++++++++++++++ internal/auth/scopes.go | 35 ++++++++++++++++++------------ 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/cmd/chat_raw.go b/cmd/chat_raw.go index 3ecf53e..f55bd4f 100644 --- a/cmd/chat_raw.go +++ b/cmd/chat_raw.go @@ -132,6 +132,13 @@ func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSi if pageSize <= 0 { pageSize = 100 } + // If the caller capped results, ask the server for at most that many + // per page. Otherwise the server's nextPageToken points past the + // full page (e.g. item 100) and clients that continue pagination + // would skip the items we silently sliced off. + if maxResults > 0 && !fetchAll && pageSize > maxResults { + pageSize = maxResults + } var aggregated *chat.ListSpacesResponse for { @@ -227,6 +234,18 @@ func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, thisPage = 1000 } } + // Clamp page size to the remaining budget so the server's + // nextPageToken stays aligned with the items we hand back. + if maxResults > 0 && !fetchAll { + collected := int64(0) + if aggregated != nil { + collected = int64(len(aggregated.Messages)) + } + remaining := maxResults - collected + if remaining > 0 && thisPage > remaining { + thisPage = remaining + } + } call := svc.Spaces.Messages.List(spaceName).PageSize(thisPage) if filter != "" { @@ -312,6 +331,13 @@ func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults = 0 } + // Clamp page size so the server's nextPageToken aligns with the + // items we return — otherwise clients continuing pagination skip + // items we sliced off locally. + if maxResults > 0 && !fetchAll && (pageSize <= 0 || pageSize > maxResults) { + pageSize = maxResults + } + if pageSize <= 0 { pageSize = maxResults if pageSize <= 0 || pageSize > 100 { diff --git a/cmd/raw_runners_test.go b/cmd/raw_runners_test.go index dc8eb53..b160db7 100644 --- a/cmd/raw_runners_test.go +++ b/cmd/raw_runners_test.go @@ -503,3 +503,50 @@ func TestParseParams_RejectsTrailingJunk(t *testing.T) { t.Fatal("expected error for trailing junk after JSON") } } + +// TestChatListRaw_ClampsPageSizeToMax verifies that --max smaller than the +// default pageSize makes the runner request just --max items per page, +// so the server's nextPageToken stays aligned with the items we return. +func TestChatListRaw_ClampsPageSizeToMax(t *testing.T) { + var seenPageSize string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seenPageSize = r.URL.Query().Get("pageSize") + w.Header().Set("Content-Type", "application/json") + // Honor the requested pageSize: return exactly 5 items and a + // next-page token positioned correctly for the next page. + _ = json.NewEncoder(w).Encode(&chat.ListSpacesResponse{ + Spaces: []*chat.Space{ + {Name: "spaces/A"}, {Name: "spaces/B"}, {Name: "spaces/C"}, + {Name: "spaces/D"}, {Name: "spaces/E"}, + }, + NextPageToken: "page-2-after-5", + }) + })) + defer server.Close() + + svc, err := chat.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(server.URL)) + if err != nil { + t.Fatal(err) + } + cmd := makeCmd(t, map[string]string{"raw": "true"}) + out, err := captureStdout(t, func() error { + // pageSize=100 default, max=5 explicit, !fetchAll → clamp to 5. + return runChatListRaw(cmd, svc, "", 100, 5, false, true) + }) + if err != nil { + t.Fatalf("runner err: %v", err) + } + if seenPageSize != "5" { + t.Errorf("expected request pageSize=5 (clamped to --max), got %q", seenPageSize) + } + var got chat.ListSpacesResponse + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatal(err) + } + if len(got.Spaces) != 5 { + t.Errorf("expected 5 spaces returned, got %d", len(got.Spaces)) + } + if got.NextPageToken != "page-2-after-5" { + t.Errorf("expected verbatim nextPageToken positioned after item 5, got %q", got.NextPageToken) + } +} diff --git a/internal/auth/scopes.go b/internal/auth/scopes.go index d94d22f..88a744b 100644 --- a/internal/auth/scopes.go +++ b/internal/auth/scopes.go @@ -6,16 +6,20 @@ const scopePrefix = "https://www.googleapis.com/auth/" // ServiceScopes maps each service to its required Google API scopes. var ServiceScopes = map[string][]string{ - "gmail": {"gmail.readonly", "gmail.send", "gmail.modify", "gmail.settings.basic", "gmail.settings.sharing"}, - "calendar": {"calendar.readonly", "calendar.events", "calendar"}, - "drive": {"drive.readonly", "drive.file", "drive"}, - "docs": {"documents.readonly", "documents"}, - "sheets": {"spreadsheets"}, - "slides": {"presentations.readonly", "presentations"}, - "tasks": {"tasks.readonly", "tasks"}, - "chat": {"chat.spaces", "chat.messages", "chat.messages.create", "chat.memberships", "chat.messages.reactions", "chat.users.readstate"}, - "forms": {"forms.responses.readonly", "forms.body", "forms.body.readonly"}, - "contacts": {"contacts.readonly", "contacts", "directory.readonly"}, + "gmail": {"gmail.readonly", "gmail.send", "gmail.modify", "gmail.settings.basic", "gmail.settings.sharing"}, + "calendar": {"calendar.readonly", "calendar.events", "calendar"}, + "drive": {"drive.readonly", "drive.file", "drive"}, + "docs": {"documents.readonly", "documents"}, + "sheets": {"spreadsheets"}, + "slides": {"presentations.readonly", "presentations"}, + "tasks": {"tasks.readonly", "tasks"}, + "chat": {"chat.spaces", "chat.messages", "chat.messages.create", "chat.memberships", "chat.messages.reactions", "chat.users.readstate"}, + "forms": {"forms.responses.readonly", "forms.body", "forms.body.readonly"}, + "contacts": {"contacts.readonly", "contacts", "directory.readonly"}, + // `people` is an alias for `contacts` — both surface the People API. + // Kept as a distinct entry so `--services people` works and scoped + // warnings can refer to either name interchangeably. + "people": {"contacts.readonly", "contacts", "directory.readonly"}, "groups": {"admin.directory.group.readonly", "admin.directory.group.member.readonly"}, "keep": {"keep.readonly", "keep"}, "driveactivity": {"drive.activity.readonly"}, @@ -70,11 +74,14 @@ func ScopesForServices(services []string) []string { } // ServiceForScope returns the service name for a given full scope URL. -// Returns empty string if the scope is not recognized. +// Returns empty string if the scope is not recognized. Iteration order is +// deterministic so aliases (e.g. `people` ↔ `contacts`) always resolve to +// the canonical name first. func ServiceForScope(scope string) string { short := strings.TrimPrefix(scope, scopePrefix) - for svc, ss := range ServiceScopes { - for _, s := range ss { + order := []string{"gmail", "calendar", "drive", "docs", "sheets", "slides", "tasks", "chat", "forms", "contacts", "people", "groups", "keep", "driveactivity", "userinfo"} + for _, svc := range order { + for _, s := range ServiceScopes[svc] { if s == short { return svc } @@ -85,7 +92,7 @@ func ServiceForScope(scope string) string { // ValidServiceNames returns all known service names. func ValidServiceNames() []string { - return []string{"gmail", "calendar", "drive", "docs", "sheets", "slides", "tasks", "chat", "forms", "contacts", "groups", "keep", "driveactivity"} + return []string{"gmail", "calendar", "drive", "docs", "sheets", "slides", "tasks", "chat", "forms", "contacts", "people", "groups", "keep", "driveactivity"} } // ValidateServices checks that all service names are recognized. From 51a153945c30e0c94e605c3d384018bff5b282b9 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 23:12:55 +0300 Subject: [PATCH 09/12] fix: address codex round-7 review on PR #189 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Gmail raw clamps perPage to remaining --max budget unconditionally. Previously the clamp only applied when --params maxResults was absent, so e.g. --max 5 --params '{"maxResults":100}' would request 100 from the server, slice to 5, and emit a nextPageToken positioned after item 100 — a client continuing pagination skipped messages 6-100. --max is now always the authoritative output cap; nextPageToken stays aligned. 2. Existing chat messages / chat members commands relaxed from ExactArgs(1) to MaximumNArgs(1). They already had --raw + --params wired, but Cobra rejected the invocation before the runner could read --params parent. Now passing `--params '{"parent":"spaces/.."}'` works on the existing commands too, matching the contacts get / gmail thread / people get pattern. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/chat.go | 22 ++++++++++++++-------- cmd/chat_test.go | 2 +- cmd/gmail.go | 6 +++++- skills/chat/references/commands.md | 4 ++-- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/cmd/chat.go b/cmd/chat.go index 12b295a..1d8a5da 100644 --- a/cmd/chat.go +++ b/cmd/chat.go @@ -38,11 +38,14 @@ var chatListCmd = &cobra.Command{ } var chatMessagesCmd = &cobra.Command{ - Use: "messages ", + Use: "messages [space-id]", Short: "List messages in a space", - Long: "Lists recent messages in a Chat space.", - Args: cobra.ExactArgs(1), - RunE: runChatMessages, + Long: `Lists recent messages in a Chat space. + +The space id is required; pass it as a positional argument or supply it +via --params (e.g. '{"parent":"spaces/AAA"}') when using --raw.`, + Args: cobra.MaximumNArgs(1), + RunE: runChatMessages, } var chatRecentCmd = &cobra.Command{ @@ -66,11 +69,14 @@ Examples: } var chatMembersCmd = &cobra.Command{ - Use: "members ", + Use: "members [space-id]", Short: "List members of a space", - Long: "Lists all members (users and bots) in a Chat space with display names.", - Args: cobra.ExactArgs(1), - RunE: runChatMembers, + Long: `Lists all members (users and bots) in a Chat space with display names. + +The space id is required; pass it as a positional argument or supply it +via --params (e.g. '{"parent":"spaces/AAA"}') when using --raw.`, + Args: cobra.MaximumNArgs(1), + RunE: runChatMembers, } var chatSendCmd = &cobra.Command{ diff --git a/cmd/chat_test.go b/cmd/chat_test.go index 07e810f..9d4f54a 100644 --- a/cmd/chat_test.go +++ b/cmd/chat_test.go @@ -93,7 +93,7 @@ func TestChatListCommand_Help(t *testing.T) { func TestChatMessagesCommand_Help(t *testing.T) { cmd := chatMessagesCmd - if cmd.Use != "messages " { + if cmd.Use != "messages [space-id]" { t.Errorf("unexpected Use: %s", cmd.Use) } if cmd.Args == nil { diff --git a/cmd/gmail.go b/cmd/gmail.go index 3cd07a9..ff8b6bc 100644 --- a/cmd/gmail.go +++ b/cmd/gmail.go @@ -669,7 +669,11 @@ func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxRe if remaining <= 0 { break } - if !paramPageSizeSet && remaining < perPage { + // Clamp every request to the remaining --max budget, even + // when --params maxResults asks for more. Otherwise the + // server's nextPageToken points past the slice we return, + // and clients continuing pagination skip items 6-N. + if remaining < perPage { perPage = remaining } } diff --git a/skills/chat/references/commands.md b/skills/chat/references/commands.md index 7f1d52c..9b36a94 100644 --- a/skills/chat/references/commands.md +++ b/skills/chat/references/commands.md @@ -65,7 +65,7 @@ gws chat spaces list --params '{"pageSize":50,"filter":"spaceType = \"DIRECT_MES Lists recent messages in a Chat space. Supports filtering, ordering, pagination, and showing deleted messages. ``` -Usage: gws chat messages [flags] +Usage: gws chat messages [space-id] [flags] ``` | Flag | Type | Default | Description | @@ -106,7 +106,7 @@ The space ID format is `spaces/AAAA1234` (get from `gws chat list`). Lists all members of a Chat space with display names and emails (auto-resolved via People API, cached locally). ``` -Usage: gws chat members [flags] +Usage: gws chat members [space-id] [flags] ``` | Flag | Type | Default | Description | From 2831d520720eb62d8dedfb0f32c560a284df66f5 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 23:20:52 +0300 Subject: [PATCH 10/12] fix: address codex round-8 review on PR #189 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. parseParams trailing-junk detection: switched from dec.More() to an explicit second-Decode-must-return-io.EOF check. The existing test passed locally — dec.More() does work for the top-level streaming case — but the new idiom doesn't rely on subtle decoder semantics and is what every reader expects. Codex marked this critical even though the test passed; the new code is unambiguously correct. 2. Raw runner errors now go through GetPrinter().PrintError so scripts get the project's structured error shape ({"error": "..."}) instead of Cobra-style stderr. Applied to runGmailListRaw, runGmailThreadRaw, runChatListRaw, runChatMessagesRaw, runChatMembersRaw, runPeopleGet, and runPeopleGetWithSvc. Updated the corresponding runner test that previously asserted on the returned error directly. 3. README command tables now show optional positional args for commands that accept the id via --params: gws gmail thread [id] gws chat messages [space] gws chat members [space] gws contacts get [resource-name] …matching the skill references and the actual Cobra Use lines. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 8 ++++---- cmd/chat_raw.go | 19 +++++++++++-------- cmd/gmail.go | 12 +++++++----- cmd/people.go | 14 ++++++++------ cmd/raw.go | 7 +++++-- cmd/raw_runners_test.go | 14 ++++++++------ 6 files changed, 43 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 4a7d0e2..637557e 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ Add `--format text` for human-readable output, or `--format yaml` for YAML. |---------|-------------| | `gws gmail list` | List threads with `thread_id` and `message_id` (`--max`, `--query`, `--all`, `--include-labels`, `--raw`, `--params`). Under `--raw` switches to `users.messages.list` shape. | | `gws gmail read ` | Read message body and headers | -| `gws gmail thread ` | Read full thread conversation (`--raw`, `--params`) | +| `gws gmail thread [id]` | Read full thread conversation (`--raw`, `--params`; id may be supplied via `--params id`) | | `gws gmail send` | Send email (`--to`, `--subject`, `--body`, `--cc`, `--bcc`, `--thread-id`, `--reply-to-message-id`, `--attachment`) | | `gws gmail reply ` | Reply to message (`--body`, `--cc`, `--bcc`, `--all`) | | `gws gmail forward ` | Forward message (`--to`, `--body`, `--cc`, `--bcc`) | @@ -350,9 +350,9 @@ Add `--format text` for human-readable output, or `--format yaml` for YAML. | `gws chat list` | List spaces (`--filter`, `--page-size`, `--raw`, `--params`) | | `gws chat spaces list` | List spaces (API-shape friendly path; same as `chat list` with `--raw` / `--params` documented examples) | | `gws chat recent` | Recap messages across active spaces (`--since`, `--max`, `--max-per-space`, `--max-spaces`) | -| `gws chat messages ` | List messages (`--max`, `--filter`, `--order-by`, `--show-deleted`, `--after`, `--before`, `--resolve-senders`, `--raw`, `--params`) | +| `gws chat messages [space]` | List messages (`--max`, `--filter`, `--order-by`, `--show-deleted`, `--after`, `--before`, `--resolve-senders`, `--raw`, `--params`; space may be supplied via `--params parent`) | | `gws chat messages list` | List messages by `parent` via `--params` (programmatic path) | -| `gws chat members ` | List members with display names + emails via People API (`--max`, `--filter`, `--show-groups`, `--show-invited`, `--raw`, `--params`) | +| `gws chat members [space]` | List members with display names + emails via People API (`--max`, `--filter`, `--show-groups`, `--show-invited`, `--raw`, `--params`; space may be supplied via `--params parent`) | | `gws chat members list` | List members by `parent` via `--params` (programmatic path) | | `gws chat send` | Send message (`--space`, `--text`) | | `gws chat get ` | Get a single message (`--resolve-senders`) | @@ -400,7 +400,7 @@ Add `--format text` for human-readable output, or `--format yaml` for YAML. |---------|-------------| | `gws contacts list` | List contacts (`--max`) | | `gws contacts search ` | Search contacts by name/email/phone | -| `gws contacts get ` | Get contact details (`--raw`, `--params`) | +| `gws contacts get [resource-name]` | Get contact details (`--raw`, `--params`; resource-name may be supplied via `--params resourceName`) | | `gws contacts create` | Create contact (`--name`, `--email`, `--phone`) | | `gws contacts delete ` | Delete a contact | diff --git a/cmd/chat_raw.go b/cmd/chat_raw.go index f55bd4f..1c86c81 100644 --- a/cmd/chat_raw.go +++ b/cmd/chat_raw.go @@ -106,9 +106,10 @@ func init() { // runChatListRaw implements `gws chat spaces list --raw` (and `gws chat list --raw`). func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSize, maxResults int64, fetchAll, maxExplicit bool) error { + p := GetPrinter() params, perr := parseParams(cmd) if perr != nil { - return perr + return p.PrintError(perr) } if v, ok := paramString(params, "filter"); ok { @@ -152,7 +153,7 @@ func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSi resp, err := call.Do() if err != nil { - return fmt.Errorf("failed to list spaces: %w", err) + return p.PrintError(fmt.Errorf("failed to list spaces: %w", err)) } if aggregated == nil { @@ -193,16 +194,17 @@ func runChatListRaw(cmd *cobra.Command, svc *chat.Service, filter string, pageSi // runChatMessagesRaw implements `gws chat messages list --raw`. func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter, orderBy string, showDeleted, fetchAll, maxExplicit bool) error { + p := GetPrinter() params, perr := parseParams(cmd) if perr != nil { - return perr + return p.PrintError(perr) } if v, ok := paramString(params, "parent"); ok && v != "" { spaceName = v } if spaceName == "" { - return errors.New("chat messages list: a space name is required (positional arg or --params parent)") + return p.PrintError(errors.New("chat messages list: a space name is required (positional arg or --params parent)")) } pageSize := int64(0) if v, ok := paramInt64(params, "pageSize"); ok { @@ -263,7 +265,7 @@ func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, resp, err := call.Do() if err != nil { - return fmt.Errorf("failed to list messages: %w", err) + return p.PrintError(fmt.Errorf("failed to list messages: %w", err)) } if aggregated == nil { @@ -299,16 +301,17 @@ func runChatMessagesRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, // runChatMembersRaw implements `gws chat members list --raw`. func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, maxResults int64, filter string, showGroups, showInvited, fetchAll, maxExplicit bool) error { + p := GetPrinter() params, perr := parseParams(cmd) if perr != nil { - return perr + return p.PrintError(perr) } if v, ok := paramString(params, "parent"); ok && v != "" { spaceName = v } if spaceName == "" { - return errors.New("chat members list: a space name is required (positional arg or --params parent)") + return p.PrintError(errors.New("chat members list: a space name is required (positional arg or --params parent)")) } pageSize := int64(0) if v, ok := paramInt64(params, "pageSize"); ok { @@ -363,7 +366,7 @@ func runChatMembersRaw(cmd *cobra.Command, svc *chat.Service, spaceName string, resp, err := call.Do() if err != nil { - return fmt.Errorf("failed to list members: %w", err) + return p.PrintError(fmt.Errorf("failed to list members: %w", err)) } if aggregated == nil { diff --git a/cmd/gmail.go b/cmd/gmail.go index ff8b6bc..a045a5b 100644 --- a/cmd/gmail.go +++ b/cmd/gmail.go @@ -611,9 +611,10 @@ func runGmailList(cmd *cobra.Command, args []string) error { // field across pages and drops nextPageToken; the final shape is // {"messages":[...], "resultSizeEstimate":N}. func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxResults int64, fetchAll, maxExplicit bool) error { + p := GetPrinter() params, perr := parseParams(cmd) if perr != nil { - return perr + return p.PrintError(perr) } // Raw mode preserves the API response verbatim. The CLI's --max // default would otherwise slice a verbatim page, breaking the @@ -694,7 +695,7 @@ func runGmailListRaw(cmd *cobra.Command, svc *gmail.Service, query string, maxRe resp, err := call.Do() if err != nil { - return fmt.Errorf("failed to list messages: %w", err) + return p.PrintError(fmt.Errorf("failed to list messages: %w", err)) } if aggregated == nil { @@ -1335,9 +1336,10 @@ func runGmailThread(cmd *cobra.Command, args []string) error { // body data in `payload.body.data` / `parts[*].body.data`, plus // `internalDate`, `labelIds`, `snippet`, etc. func runGmailThreadRaw(cmd *cobra.Command, svc *gmail.Service, threadID string) error { + p := GetPrinter() params, perr := parseParams(cmd) if perr != nil { - return perr + return p.PrintError(perr) } // --params overrides: id (resource path), format, metadataHeaders. @@ -1345,7 +1347,7 @@ func runGmailThreadRaw(cmd *cobra.Command, svc *gmail.Service, threadID string) threadID = v } if threadID == "" { - return fmt.Errorf("gmail thread: a thread id is required (positional arg or --params id)") + return p.PrintError(fmt.Errorf("gmail thread: a thread id is required (positional arg or --params id)")) } format := "full" if v, ok := paramString(params, "format"); ok && v != "" { @@ -1360,7 +1362,7 @@ func runGmailThreadRaw(cmd *cobra.Command, svc *gmail.Service, threadID string) thread, err := call.Do() if err != nil { - return fmt.Errorf("failed to get thread: %w", err) + return p.PrintError(fmt.Errorf("failed to get thread: %w", err)) } return printRaw(thread) } diff --git a/cmd/people.go b/cmd/people.go index 1063f83..9f227a1 100644 --- a/cmd/people.go +++ b/cmd/people.go @@ -51,14 +51,15 @@ func init() { } func runPeopleGet(cmd *cobra.Command, args []string) error { + p := GetPrinter() ctx := context.Background() factory, err := client.NewFactory(ctx) if err != nil { - return err + return p.PrintError(err) } svc, err := factory.People() if err != nil { - return err + return p.PrintError(err) } return runPeopleGetWithSvc(cmd, svc, args) } @@ -67,9 +68,10 @@ func runPeopleGet(cmd *cobra.Command, args []string) error { // an injected *people.Service so the runner can be exercised against an // httptest backend without needing real OAuth. func runPeopleGetWithSvc(cmd *cobra.Command, svc *people.Service, args []string) error { + p := GetPrinter() params, perr := parseParams(cmd) if perr != nil { - return perr + return p.PrintError(perr) } resourceName := "" @@ -80,7 +82,7 @@ func runPeopleGetWithSvc(cmd *cobra.Command, svc *people.Service, args []string) resourceName = v } if resourceName == "" { - return errors.New("people get: resourceName is required (positional arg or --params resourceName)") + return p.PrintError(errors.New("people get: resourceName is required (positional arg or --params resourceName)")) } pf, _ := cmd.Flags().GetString("person-fields") @@ -101,11 +103,11 @@ func runPeopleGetWithSvc(cmd *cobra.Command, svc *people.Service, args []string) person, err := call.Do() if err != nil { - return fmt.Errorf("failed to get person: %w", err) + return p.PrintError(fmt.Errorf("failed to get person: %w", err)) } if isRaw(cmd) { return printRaw(person) } - return GetPrinter().Print(formatPerson(person)) + return p.Print(formatPerson(person)) } diff --git a/cmd/raw.go b/cmd/raw.go index 5df07c7..9ecae59 100644 --- a/cmd/raw.go +++ b/cmd/raw.go @@ -17,6 +17,7 @@ package cmd import ( "encoding/json" + "errors" "fmt" "io" "os" @@ -63,8 +64,10 @@ func parseParams(cmd *cobra.Command) (map[string]interface{}, error) { return nil, fmt.Errorf("--params: invalid JSON: %w", err) } // Reject trailing junk after the object so scripts don't silently - // drop a typo'd second value. - if dec.More() { + // drop a typo'd second value. A second Decode must return io.EOF; + // anything else (another token, syntax error) is junk. + var extra interface{} + if err := dec.Decode(&extra); !errors.Is(err, io.EOF) { return nil, fmt.Errorf("--params: unexpected trailing data after JSON object") } return m, nil diff --git a/cmd/raw_runners_test.go b/cmd/raw_runners_test.go index b160db7..7300478 100644 --- a/cmd/raw_runners_test.go +++ b/cmd/raw_runners_test.go @@ -488,12 +488,14 @@ func TestRunPeopleGet_MissingResourceNameErrors(t *testing.T) { defer cleanup() cmd := newPeopleGetCmd(t, nil) - err := runPeopleGetWithSvc(cmd, svc, nil) - if err == nil { - t.Fatal("expected error when resourceName missing") - } - if !strings.Contains(err.Error(), "resourceName is required") { - t.Errorf("unexpected error message: %v", err) + // Per the printer-error convention, the runner emits a structured + // error object via the printer and returns the printer's nil. The + // error message must show up on stdout. + out, _ := captureStdout(t, func() error { + return runPeopleGetWithSvc(cmd, svc, nil) + }) + if !strings.Contains(out, "resourceName is required") { + t.Errorf("expected structured error JSON on stdout, got %q", out) } } From a9d7f30b042c82d1178f90a7fa6348a9da46c235 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 23:27:03 +0300 Subject: [PATCH 11/12] fix: address codex round-9 review on PR #189 1. Validate resource ids before auth/factory for optional-positional commands. Previously `gws gmail thread` (no id, no --params id) hit OAuth/config errors before reporting the missing id. Now each affected runner parses --params and resolves the id from positional or params, validates non-empty, and only then creates the client. Affected runners: runGmailThread, runContactsGet, runPeopleGet, runChatMessages, runChatMembers. 2. README pagination doc now lists the correct API field for chat members: the response field is `memberships`, not `members` (the code/tests already used the right name; only the doc was off). Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 2 +- cmd/chat.go | 49 ++++++++++++++++++++++++++++++++----------------- cmd/contacts.go | 24 +++++++++++++----------- cmd/gmail.go | 31 ++++++++++++++++++++----------- cmd/people.go | 16 ++++++++++++++++ 5 files changed, 82 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 637557e..a99bf6b 100644 --- a/README.md +++ b/README.md @@ -477,7 +477,7 @@ Several list/get commands accept two extra flags for scripting: equivalent CLI flags (params win). Under `--all`, raw mode concatenates the top-level list field across pages -(messages / spaces / members) and drops `nextPageToken` from the final +(`messages` / `spaces` / `memberships`) and drops `nextPageToken` from the final output. Supported in this release: diff --git a/cmd/chat.go b/cmd/chat.go index 1d8a5da..544f570 100644 --- a/cmd/chat.go +++ b/cmd/chat.go @@ -638,8 +638,23 @@ func runChatList(cmd *cobra.Command, args []string) error { func runChatMessages(cmd *cobra.Command, args []string) error { p := GetPrinter() - ctx := context.Background() + // Resolve space name from positional + --params before touching auth + // so input errors surface ahead of OAuth/config errors. + spaceName := "" + if len(args) > 0 { + spaceName = ensureSpaceName(args[0]) + } + if params, perr := parseParams(cmd); perr != nil { + return p.PrintError(perr) + } else if v, ok := paramString(params, "parent"); ok && v != "" { + spaceName = v + } + if spaceName == "" { + return p.PrintError(errors.New("chat messages: a space id is required (positional arg or --params parent)")) + } + + ctx := context.Background() factory, err := client.NewFactory(ctx) if err != nil { return p.PrintError(err) @@ -649,11 +664,6 @@ func runChatMessages(cmd *cobra.Command, args []string) error { if err != nil { return p.PrintError(err) } - - spaceName := "" - if len(args) > 0 { - spaceName = ensureSpaceName(args[0]) - } maxResults, _ := cmd.Flags().GetInt64("max") filter, _ := cmd.Flags().GetString("filter") orderBy, _ := cmd.Flags().GetString("order-by") @@ -686,9 +696,6 @@ func runChatMessages(cmd *cobra.Command, args []string) error { if isRaw(cmd) { return runChatMessagesRaw(cmd, svc, spaceName, maxResults, filter, orderBy, showDeleted, fetchAll, cmd.Flags().Changed("max")) } - if spaceName == "" { - return p.PrintError(errors.New("chat messages: a space id is required")) - } if maxResults <= 0 { return p.Print(map[string]interface{}{ @@ -1045,8 +1052,23 @@ func runChatRecent(cmd *cobra.Command, args []string) error { func runChatMembers(cmd *cobra.Command, args []string) error { p := GetPrinter() - ctx := context.Background() + // Resolve space name from positional + --params before touching auth + // so input errors surface ahead of OAuth/config errors. + spaceName := "" + if len(args) > 0 { + spaceName = ensureSpaceName(args[0]) + } + if params, perr := parseParams(cmd); perr != nil { + return p.PrintError(perr) + } else if v, ok := paramString(params, "parent"); ok && v != "" { + spaceName = v + } + if spaceName == "" { + return p.PrintError(errors.New("chat members: a space id is required (positional arg or --params parent)")) + } + + ctx := context.Background() factory, err := client.NewFactory(ctx) if err != nil { return p.PrintError(err) @@ -1057,10 +1079,6 @@ func runChatMembers(cmd *cobra.Command, args []string) error { return p.PrintError(err) } - spaceName := "" - if len(args) > 0 { - spaceName = ensureSpaceName(args[0]) - } maxResults, _ := cmd.Flags().GetInt64("max") filter, _ := cmd.Flags().GetString("filter") showGroups, _ := cmd.Flags().GetBool("show-groups") @@ -1073,9 +1091,6 @@ func runChatMembers(cmd *cobra.Command, args []string) error { if isRaw(cmd) { return runChatMembersRaw(cmd, svc, spaceName, maxResults, filter, showGroups, showInvited, fetchAll, cmd.Flags().Changed("max")) } - if spaceName == "" { - return p.PrintError(errors.New("chat members: a space id is required")) - } // Page size per request (Google caps at 100) pageSize := maxResults diff --git a/cmd/contacts.go b/cmd/contacts.go index bf5246f..8d68cd9 100644 --- a/cmd/contacts.go +++ b/cmd/contacts.go @@ -290,18 +290,9 @@ func runContactsSearch(cmd *cobra.Command, args []string) error { func runContactsGet(cmd *cobra.Command, args []string) error { p := GetPrinter() - ctx := context.Background() - - factory, err := client.NewFactory(ctx) - if err != nil { - return p.PrintError(err) - } - - svc, err := factory.People() - if err != nil { - return p.PrintError(err) - } + // Resolve resource name from positional + --params before touching auth + // so input errors surface ahead of OAuth/config errors. resourceName := "" if len(args) > 0 { resourceName = args[0] @@ -322,6 +313,17 @@ func runContactsGet(cmd *cobra.Command, args []string) error { return p.PrintError(fmt.Errorf("contacts get: resource-name is required (positional arg or --params resourceName)")) } + ctx := context.Background() + factory, err := client.NewFactory(ctx) + if err != nil { + return p.PrintError(err) + } + + svc, err := factory.People() + if err != nil { + return p.PrintError(err) + } + call := svc.People.Get(resourceName).PersonFields(pf) if sources, ok := paramStringSlice(params, "sources"); ok && len(sources) > 0 { call = call.Sources(sources...) diff --git a/cmd/gmail.go b/cmd/gmail.go index a045a5b..97690a7 100644 --- a/cmd/gmail.go +++ b/cmd/gmail.go @@ -1262,8 +1262,25 @@ func runGmailTrash(cmd *cobra.Command, args []string) error { func runGmailThread(cmd *cobra.Command, args []string) error { p := GetPrinter() - ctx := context.Background() + // Resolve thread id from positional + --params before touching auth + // so input errors surface ahead of OAuth/config errors. + threadID := "" + if len(args) > 0 { + threadID = args[0] + } + params, perr := parseParams(cmd) + if perr != nil { + return p.PrintError(perr) + } + if v, ok := paramString(params, "id"); ok && v != "" { + threadID = v + } + if threadID == "" { + return p.PrintError(fmt.Errorf("gmail thread: a thread id is required (positional arg or --params id)")) + } + + ctx := context.Background() factory, err := client.NewFactory(ctx) if err != nil { return p.PrintError(err) @@ -1274,19 +1291,11 @@ func runGmailThread(cmd *cobra.Command, args []string) error { return p.PrintError(err) } - threadID := "" - if len(args) > 0 { - threadID = args[0] - } - if isRaw(cmd) { - // runGmailThreadRaw applies --params id; final emptiness is - // validated inside the raw runner. + // runGmailThreadRaw also parses --params; threadID is already + // resolved, so the runner's own params check is a no-op pass-through. return runGmailThreadRaw(cmd, svc, threadID) } - if threadID == "" { - return p.PrintError(fmt.Errorf("gmail thread: a thread id is required")) - } thread, err := svc.Users.Threads.Get("me", threadID).Format("full").Do() if err != nil { diff --git a/cmd/people.go b/cmd/people.go index 9f227a1..c8c90dd 100644 --- a/cmd/people.go +++ b/cmd/people.go @@ -52,6 +52,22 @@ func init() { func runPeopleGet(cmd *cobra.Command, args []string) error { p := GetPrinter() + + // Pre-validate that we have a resourceName from positional or + // --params before touching auth. Surfaces input errors immediately + // instead of after OAuth/config failures. + params, perr := parseParams(cmd) + if perr != nil { + return p.PrintError(perr) + } + hasResource := len(args) > 0 && args[0] != "" + if v, ok := paramString(params, "resourceName"); ok && v != "" { + hasResource = true + } + if !hasResource { + return p.PrintError(errors.New("people get: resourceName is required (positional arg or --params resourceName)")) + } + ctx := context.Background() factory, err := client.NewFactory(ctx) if err != nil { From 750d88f4a57ec6862d6ea2a7c934e83e2193ddb0 Mon Sep 17 00:00:00 2001 From: Omri Ariav Date: Wed, 13 May 2026 23:32:08 +0300 Subject: [PATCH 12/12] fix: address codex round-10 review on PR #189 Factory scope check now treats `people` and `contacts` as alias services. Previously `gws auth login --services people` granted the right OAuth scopes (via the auth.ServiceScopes alias added in round 6), but `factory.People()` still warned "contacts requires additional permissions" because the check looked for an exact name match in grantedServices. Introduced a small serviceAliases map + canonicalService() helper so any future aliases resolve through the same path. Test added. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/client/factory.go | 24 ++++++++++++++++++++++-- internal/client/factory_test.go | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/internal/client/factory.go b/internal/client/factory.go index 818c951..6a779e9 100644 --- a/internal/client/factory.go +++ b/internal/client/factory.go @@ -84,16 +84,27 @@ func NewFactory(ctx context.Context) (*Factory, error) { }, nil } +// serviceAliases maps every accepted alias for a service to its canonical +// name. Aliases share the same OAuth scope set in auth.ServiceScopes — see +// `people` ↔ `contacts` (both surface the People API). +var serviceAliases = map[string]string{ + "contacts": "contacts", + "people": "contacts", +} + // checkServiceScopes checks if the required service was granted during login. // Prints a warning to stderr if the service was not included in the scoped login. +// Treats `people` and `contacts` (and any other entry in serviceAliases) as +// equivalent — a login with one grants the other. func (f *Factory) checkServiceScopes(service string) { if len(f.grantedServices) == 0 { return // Full auth or no metadata — skip check } + canonical := canonicalService(service) for _, s := range f.grantedServices { - if s == service { - return // Granted + if canonicalService(s) == canonical { + return // Granted (directly or via alias) } } @@ -108,6 +119,15 @@ func (f *Factory) checkServiceScopes(service string) { service, strings.Join(allServices, ",")) } +// canonicalService returns the canonical name for a service, resolving any +// alias (e.g. "people" → "contacts"). Unknown services pass through. +func canonicalService(s string) string { + if c, ok := serviceAliases[s]; ok { + return c + } + return s +} + // Gmail returns the Gmail service client. func (f *Factory) Gmail() (*gmail.Service, error) { f.mu.Lock() diff --git a/internal/client/factory_test.go b/internal/client/factory_test.go index f1d003f..b7a4b4d 100644 --- a/internal/client/factory_test.go +++ b/internal/client/factory_test.go @@ -71,6 +71,27 @@ func TestPeople_WarnsOnMissingContactsScope(t *testing.T) { } } +// TestPeople_PeopleAliasGrantsContactsScope verifies that users who run +// `gws auth login --services people` don't see the misleading "contacts +// requires additional permissions" warning on `gws people get` (or any +// other path that hits factory.People). The two service names share the +// same OAuth scope set per internal/auth/scopes.go. +func TestPeople_PeopleAliasGrantsContactsScope(t *testing.T) { + f := &Factory{ + ctx: context.Background(), + grantedServices: []string{"people"}, + scopeWarned: map[string]bool{}, + mu: sync.Mutex{}, + } + + out := captureStderr(t, func() { + _, _ = f.People() + }) + if strings.Contains(out, "requires additional permissions") { + t.Errorf("--services people must satisfy the contacts scope check; got %q", out) + } +} + func TestPeopleProfile_NoWarnEvenAfterPeopleWarned(t *testing.T) { // Mixed flow: a code path may have already used People() (and warned). // PeopleProfile should still not emit a warning of its own.