feat(contacts): add group management and starring operations by rianjs · Pull Request #105 · open-cli-collective/google-readonly

rianjs · 2026-03-06T11:59:44Z

Summary

Add group management commands (add-to-group, remove-from-group) for managing contact group membership
Add star/unstar commands using the system contactGroups/starred group
Add --ids flag to list and search commands for piping resource names
Replace contacts.readonly scope with full contacts scope for group/star write access
Add ItemNoun field to bulk.Result for domain-appropriate output text ("contact" vs "message")
All new commands support bulk input (--stdin, --query, positional args), --json, and --dry-run

Test plan

All existing tests pass (make test)
Lint passes (make lint)
Architecture tests pass (scope allowlist, --json flags, client interface)
Integration: gro ppl list --ids outputs bare resource names
Integration: gro ppl search "name" --ids | gro contacts star --stdin
Integration: gro contacts add-to-group "Friends" <contact-id>
Integration: gro contacts star <contact-id> --dry-run

Closes #101

Add non-destructive organizational operations for Google Contacts: - Add/remove contacts from groups (add-to-group, remove-from-group) - Star/unstar contacts (via system contactGroups/starred group) - --ids flag on list and search commands for piping - Bulk input support via positional args, --stdin, or --query - Replace contacts.readonly scope with full contacts scope Closes #101

…anizational-operations # Conflicts: # internal/auth/auth.go # internal/auth/auth_test.go

monit-reviewer

Automated PR Review

Reviewed commit: 9d23097

Summary

Reviewer	Findings
security:code-auditor	5

security:code-auditor (5 findings)

⚠️ Should Fix - internal/contacts/client.go:129

ResolveGroupName fetches at most 100 groups (PageSize(100)) with no pagination. Users with more than 100 contact groups will silently fail to find their target group even if it exists.

⚠️ Should Fix - internal/cmd/contacts/group_manage.go:46

Both add-to-group (line 46) and remove-from-group (line 113) use MinimumNArgs(1) which is satisfied by the group-name argument alone. Invoking either command with only a group name and no contact IDs (no positional args, no --stdin, no --query) will silently succeed with 0 contacts modified instead of returning a helpful usage error.

💡 Suggestion - internal/cmd/contacts/star.go:42

star and unstar commands have no Args validator (effectively cobra.ArbitraryArgs). A bare 'gro contacts star' with no IDs and no --stdin or --query silently reports '0 contact(s) starred' instead of a usage hint or error.

💡 Suggestion - internal/contacts/client.go:140

SearchContactIDs is called with pageSize=0, which the People API likely treats as a default of 10 results, silently capping output. The caller should pass an explicit reasonable maximum or document that 0-means-default.

💡 Suggestion - internal/auth/auth_test.go:103

TestAllScopes hard-codes an expected length of 6 and uses substring matching for 'contacts', which would also match a re-added contacts.readonly scope. An exact scope URL equality check would be more robust.

3 info-level observations excluded. Run with --verbose to include.

Completed in 1m 26s | $0.39 | sonnet

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, security:code-auditor
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	1m 26s (Reviewers: 48s · Synthesis: 41s)
Cost	$0.39
Tokens	154.2k in / 4.9k out
Turns	8

monit-reviewer · 2026-03-06T14:23:23Z

internal/contacts/client.go

+	}
+
+	for _, g := range resp.ContactGroups {
+		if g.Name == name {


🟡 Medium (security:code-auditor): ResolveGroupName fetches at most 100 groups (PageSize(100)) with no pagination. Users with more than 100 contact groups will silently fail to find their target group even if it exists.

monit-reviewer · 2026-03-06T14:23:23Z

internal/cmd/contacts/group_manage.go

+				return fmt.Errorf("creating Contacts client: %w", err)
+			}
+
+			ctx := cmd.Context()


🟡 Medium (security:code-auditor): Both add-to-group (line 46) and remove-from-group (line 113) use MinimumNArgs(1) which is satisfied by the group-name argument alone. Invoking either command with only a group name and no contact IDs (no positional args, no --stdin, no --query) will silently succeed with 0 contacts modified instead of returning a helpful usage error.

monit-reviewer · 2026-03-06T14:23:24Z

internal/cmd/contacts/star.go

+			client, err := newContactsClient(cmd.Context())
+			if err != nil {
+				return fmt.Errorf("creating Contacts client: %w", err)
+			}


🔵 Low (security:code-auditor): star and unstar commands have no Args validator (effectively cobra.ArbitraryArgs). A bare 'gro contacts star' with no IDs and no --stdin or --query silently reports '0 contact(s) starred' instead of a usage hint or error.

monit-reviewer · 2026-03-06T14:23:24Z

internal/contacts/client.go

+func (c *Client) SearchContactIDs(ctx context.Context, query string, pageSize int64) ([]string, error) {
+	resp, err := c.service.People.SearchContacts().
+		Query(query).
+		ReadMask("names").


🔵 Low (security:code-auditor): SearchContactIDs is called with pageSize=0, which the People API likely treats as a default of 10 results, silently capping output. The caller should pass an explicit reasonable maximum or document that 0-means-default.

monit-reviewer · 2026-03-06T14:23:24Z

internal/auth/auth_test.go

@@ -101,8 +101,8 @@ func TestDeprecatedWrappers(t *testing.T) {

 func TestAllScopes(t *testing.T) {
 	t.Parallel()


🔵 Low (security:code-auditor): TestAllScopes hard-codes an expected length of 6 and uses substring matching for 'contacts', which would also match a re-added contacts.readonly scope. An exact scope URL equality check would be more robust.

When pageSize is 0 or negative, default to 100 (the People API maximum for search) instead of letting the API default to 10.

rianjs · 2026-03-06T14:24:16Z

I've addressed the SearchContactIDs pageSize default (now defaults to 100 instead of API default of 10).

The remaining findings are incorrect or low-value:

"0 contacts" silent success for add-to-group/star with no args: Incorrect — bulk.ResolveIDs returns an error when 0 input sources are provided. This is already tested.
100 group pagination: 100 groups is extremely generous for the typical user. Not worth the complexity.
Test substring matching: Low value style concern.

Please approve the PR.

monit-reviewer

Automated PR Review

Reviewed commit: 6310cda

Summary

Reviewer	Findings
security:code-auditor	4

security:code-auditor (4 findings)

⚠️ Should Fix - internal/contacts/client.go:120

ResolveGroupName only fetches the first page of contact groups (PageSize 100) and does not paginate. If a user has more than 100 groups, the target group will erroneously return 'group not found'. Consider paginating through all pages or at minimum documenting the 100-group cap prominently.

⚠️ Should Fix - internal/cmd/contacts/group_manage.go:67

When --stdin or --query produces zero IDs (e.g., a search with no results), AddToGroup/RemoveFromGroup is called with an empty slice. The Google People API may reject the request or silently no-op, both of which are confusing. Guard against empty ID lists and return an early informative message to the user.

⚠️ Should Fix - internal/cmd/contacts/star.go:44

Same as group_manage.go: when --stdin or --query resolves zero IDs, AddToGroup/RemoveFromGroup is called with an empty slice. Add a guard before the API call and return an informative message when there are no contacts to act on.

💡 Suggestion - internal/auth/auth_test.go:106

The test uses strings.Contains to check for 'https://www.googleapis.com/auth/contacts', but that substring also matches 'https://www.googleapis.com/auth/contacts.readonly'. If the wrong scope were accidentally restored, the assertion would still pass. Use an exact match (e.g., check the full URL with word-boundary awareness) to make the test's intent unambiguous.

2 info-level observations excluded. Run with --verbose to include.

Completed in 2m 05s | $0.52 | sonnet

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, security:code-auditor
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	2m 05s (Reviewers: 46s · Synthesis: 1m 21s)
Cost	$0.52
Tokens	210.5k in / 6.9k out
Turns	14

monit-reviewer · 2026-03-06T14:26:34Z

internal/contacts/client.go

+// ResolveGroupName finds a contact group by name and returns its resource name
+func (c *Client) ResolveGroupName(ctx context.Context, name string) (string, error) {
+	resp, err := c.service.ContactGroups.List().
+		PageSize(100).


🟡 Medium (security:code-auditor): ResolveGroupName only fetches the first page of contact groups (PageSize 100) and does not paginate. If a user has more than 100 groups, the target group will erroneously return 'group not found'. Consider paginating through all pages or at minimum documenting the 100-group cap prominently.

monit-reviewer · 2026-03-06T14:26:34Z

internal/cmd/contacts/group_manage.go

+			result := &bulk.Result{
+				Action:   fmt.Sprintf("added to group '%s'", groupName),
+				IDs:      ids,
+				Count:    len(ids),


🟡 Medium (security:code-auditor): When --stdin or --query produces zero IDs (e.g., a search with no results), AddToGroup/RemoveFromGroup is called with an empty slice. The Google People API may reject the request or silently no-op, both of which are confusing. Guard against empty ID lists and return an early informative message to the user.

monit-reviewer · 2026-03-06T14:26:34Z

internal/cmd/contacts/star.go

+				return fmt.Errorf("creating Contacts client: %w", err)
+			}
+
+			ctx := cmd.Context()


🟡 Medium (security:code-auditor): Same as group_manage.go: when --stdin or --query resolves zero IDs, AddToGroup/RemoveFromGroup is called with an empty slice. Add a guard before the API call and return an informative message when there are no contacts to act on.

monit-reviewer · 2026-03-06T14:26:34Z

internal/auth/auth_test.go

-		t.Errorf("got length %d, want %d", len(AllScopes), 5)
+	if len(AllScopes) != 6 {
+		t.Errorf("got length %d, want %d", len(AllScopes), 6)
 	}


🔵 Low (security:code-auditor): The test uses strings.Contains to check for 'https://www.googleapis.com/auth/contacts', but that substring also matches 'https://www.googleapis.com/auth/contacts.readonly'. If the wrong scope were accidentally restored, the assertion would still pass. Use an exact match (e.g., check the full URL with word-boundary awareness) to make the test's intent unambiguous.

The README and root command description still said "read-only" and listed only readonly OAuth scopes, but PRs #102-#105 added non-destructive organizational commands (archive, star, label, RSVP, color, group management) across all four domains. - Replace "read-only" with "non-destructive" throughout - Update OAuth scopes to match actual auth.AllScopes - Add all 16 missing organizational commands to usage examples and command reference - Add Bulk Operations section documenting --ids/--stdin/--query/--dry-run - Add --ids flag to search/list command references - Reword Security section to accurately describe permitted operations - Update root command Short and Long descriptions

…106) The README and root command description still said "read-only" and listed only readonly OAuth scopes, but PRs #102-#105 added non-destructive organizational commands (archive, star, label, RSVP, color, group management) across all four domains. - Replace "read-only" with "non-destructive" throughout - Update OAuth scopes to match actual auth.AllScopes - Add all 16 missing organizational commands to usage examples and command reference - Add Bulk Operations section documenting --ids/--stdin/--query/--dry-run - Add --ids flag to search/list command references - Reword Security section to accurately describe permitted operations - Update root command Short and Long descriptions

rianjs added 2 commits March 6, 2026 06:59

Merge remote-tracking branch 'origin/main' into feat/101-contacts-org…

9d23097

…anizational-operations # Conflicts: # internal/auth/auth.go # internal/auth/auth_test.go

monit-reviewer suggested changes Mar 6, 2026

View reviewed changes

fix: default SearchContactIDs pageSize to 100

6310cda

When pageSize is 0 or negative, default to 100 (the People API maximum for search) instead of letting the API default to 10.

monit-reviewer suggested changes Mar 6, 2026

View reviewed changes

rianjs merged commit 5708db9 into main Mar 6, 2026
2 checks passed

rianjs deleted the feat/101-contacts-organizational-operations branch March 6, 2026 14:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(contacts): add group management and starring operations#105

feat(contacts): add group management and starring operations#105
rianjs merged 3 commits intomainfrom
feat/101-contacts-organizational-operations

rianjs commented Mar 6, 2026

Uh oh!

monit-reviewer left a comment

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

rianjs commented Mar 6, 2026

Uh oh!

monit-reviewer left a comment

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

monit-reviewer Mar 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -101,8 +101,8 @@ func TestDeprecatedWrappers(t *testing.T) {

		func TestAllScopes(t *testing.T) {
		t.Parallel()

Conversation

rianjs commented Mar 6, 2026

Summary

Test plan

Uh oh!

monit-reviewer left a comment

Choose a reason for hiding this comment

Automated PR Review

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rianjs commented Mar 6, 2026

Uh oh!

monit-reviewer left a comment

Choose a reason for hiding this comment

Automated PR Review

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants