feat(calendar): add RSVP and color-coding operations by rianjs · Pull Request #103 · open-cli-collective/google-readonly

rianjs · 2026-03-06T11:42:16Z

Summary

Add gro cal rsvp <event-id> <accept|decline|tentative> for updating RSVP status
Add gro cal color <event-id> <color> for setting event colors (by name or ID 1-11)
Add calendar.CalendarEventsScope to OAuth scopes for event write access
Both commands support --calendar, --json/-j, and --dry-run/-n flags

Details

Scope change: Adds calendar.events scope alongside existing calendar.readonly. The events scope enables RSVP and color operations but doesn't cover calendar list metadata, so both scopes are needed.

RSVP command: Fetches the event, finds the current user's attendee entry (via Self field), updates ResponseStatus, and patches the event. Validates input to accept/decline/tentative.

Color command: Patches the event's ColorId field. Accepts Google Calendar's 11 named colors (lavender, sage, grape, flamingo, banana, tangerine, peacock, graphite, blueberry, basil, tomato) or numeric IDs 1-11.

Architecture: Both commands skip API calls entirely in dry-run mode. Architecture tests updated with calendar.events in the scope allowlist.

Test plan

All existing tests pass (make test — 23 packages)
Lint clean (make lint — 0 issues)
Architecture tests pass (scope allowlist, JSON flag, client interface, etc.)
RSVP tests: accept/decline/tentative, invalid input, dry-run, JSON, --calendar flag, case insensitive, API error, client error
Color tests: by name, by ID, invalid color, invalid numeric ID, dry-run, JSON, API error, client error, case insensitive, resolveColor unit tests

Closes #99

Add two new calendar commands for non-destructive event operations: - `gro cal rsvp <event-id> <accept|decline|tentative>` — update RSVP status - `gro cal color <event-id> <color>` — set event color by name or ID (1-11) Both support --calendar, --json, and --dry-run flags. Adds calendar.CalendarEventsScope to OAuth scopes (kept alongside CalendarReadonlyScope since events scope doesn't cover calendar list metadata). Closes #99

monit-reviewer

Automated PR Review

Reviewed commit: 8c907e6

Summary

Reviewer	Findings
security:code-auditor	4

security:code-auditor (4 findings)

⚠️ Should Fix - internal/calendar/client.go:103

Double error wrapping for RSVP: RSVPEvent wraps the patch error with "updating RSVP: %w", and the caller in rsvp.go:65 also wraps with the same prefix. The resulting error reads "updating RSVP: updating RSVP: <api error>". Remove the wrapping in one layer — convention in this codebase is for the client method to own the wrapping context.

⚠️ Should Fix - internal/calendar/client.go:115

Double error wrapping for color: SetEventColor wraps with "setting event color: %w", and the caller in color.go:94 wraps with the same prefix, producing "setting event color: setting event color: <api error>". Remove the wrapping in one layer.

💡 Suggestion - internal/calendar/client.go:101

RSVPEvent calls SendUpdates("none"), which suppresses all notification emails — the event organizer will not be notified of the RSVP response. This is likely unexpected end-user behavior. Consider omitting SendUpdates to use the API default, or at minimum document the suppression prominently in the command's help text.

💡 Suggestion - internal/cmd/calendar/rsvp.go:47

Inconsistent action field in JSON output: dry-run produces "RSVP '<response>' to event" while the live path produces "RSVP'd '<response>' to event". Programmatic consumers diffing dry-run vs. live output will see divergent strings. Normalize to one form.

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

Completed in 3m 44s | $0.71 | sonnet

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, security:code-auditor
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	3m 44s (Reviewers: 3m 07s · Synthesis: 39s)
Cost	$0.71
Tokens	60.4k in / 17.8k out
Turns	5

monit-reviewer · 2026-03-06T11:46:13Z

internal/calendar/client.go

+		Attendees: event.Attendees,
+	}).SendUpdates("none").Context(ctx).Do()
+	if err != nil {
+		return fmt.Errorf("updating RSVP: %w", err)


🟡 Medium (security:code-auditor): Double error wrapping for RSVP: RSVPEvent wraps the patch error with "updating RSVP: %w", and the caller in rsvp.go:65 also wraps with the same prefix. The resulting error reads "updating RSVP: updating RSVP: <api error>". Remove the wrapping in one layer — convention in this codebase is for the client method to own the wrapping context.

monit-reviewer · 2026-03-06T11:46:13Z

internal/calendar/client.go

+		ColorId: colorID,
+	}).Context(ctx).Do()
+	if err != nil {
+		return fmt.Errorf("setting event color: %w", err)


🟡 Medium (security:code-auditor): Double error wrapping for color: SetEventColor wraps with "setting event color: %w", and the caller in color.go:94 wraps with the same prefix, producing "setting event color: setting event color: <api error>". Remove the wrapping in one layer.

monit-reviewer · 2026-03-06T11:46:13Z

internal/calendar/client.go

+
+	_, err = c.service.Events.Patch(calendarID, eventID, &calendar.Event{
+		Attendees: event.Attendees,
+	}).SendUpdates("none").Context(ctx).Do()


🔵 Low (security:code-auditor): RSVPEvent calls SendUpdates("none"), which suppresses all notification emails — the event organizer will not be notified of the RSVP response. This is likely unexpected end-user behavior. Consider omitting SendUpdates to use the API default, or at minimum document the suppression prominently in the command's help text.

monit-reviewer · 2026-03-06T11:46:13Z

internal/cmd/calendar/rsvp.go

+
+			if dryRun {
+				result := map[string]any{
+					"action":   fmt.Sprintf("RSVP '%s' to event", apiResponse),


🔵 Low (security:code-auditor): Inconsistent action field in JSON output: dry-run produces "RSVP '<response>' to event" while the live path produces "RSVP'd '<response>' to event". Programmatic consumers diffing dry-run vs. live output will see divergent strings. Normalize to one form.

- Remove error wrapping in RSVPEvent/SetEventColor client methods (callers already provide context via their own wrapping) - Remove SendUpdates("none") from RSVPEvent to use API default behavior (organizer will be notified of RSVP changes)

monit-reviewer

Automated PR Review

Reviewed commit: 70f2f47

Summary

Reviewer	Findings
security:code-auditor	6

security:code-auditor (6 findings)

⚠️ Should Fix - internal/auth/auth.go:25

calendar.CalendarEventsScope (calendar.events) permits creating new calendar events, not just updating existing ones. The project's non-destructive philosophy allows organizational modifications but not content creation. Given this trade-off is consciously accepted, it should be explicitly documented in a comment on the scope entry (e.g., 'note: this scope also permits event creation, which is an accepted trade-off for RSVP/color support').

💡 Suggestion - internal/calendar/client.go:78

RSVPEvent accepts any string as the response parameter without validation — validation is enforced only in the cmd layer. Since this is an internal package, the risk is low, but any direct caller (tests, future packages) can pass arbitrary values to the Calendar API. Consider validating against the allowed set at the client layer for defense in depth.

💡 Suggestion - internal/calendar/client.go:101

RSVPEvent returns the Patch call error unwrapped (bare return err), inconsistent with the preceding Get error which is properly wrapped with fmt.Errorf. SetEventColor has the same issue at line 110. Per project conventions, errors should be wrapped with %w for context.

💡 Suggestion - internal/cmd/calendar/rsvp.go:47

The 'action' field in the dry-run JSON result uses present tense ("RSVP '%s' to event") while the success result at line 68 uses past tense ("RSVP'd '%s' to event"). This tense inconsistency between dry-run and non-dry-run output may confuse downstream consumers parsing the JSON.

💡 Suggestion - internal/cmd/calendar/color.go:107

resolveColor does not normalize its input to lowercase and only works correctly because newColorCommand calls strings.ToLower before invoking it. Either lowercase internally or add a comment requiring pre-normalized input to prevent breakage if the function is called from elsewhere.

💡 Suggestion - internal/cmd/calendar/color.go:44

The sorted colorNames slice is constructed and sorted twice: once in newColorCommand for help text and again in resolveColor for the error message. A package-level variable would eliminate the duplication and the redundant sort.Strings calls.

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

Completed in 2m 57s | $0.51 | sonnet

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, security:code-auditor
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	2m 57s (Reviewers: 2m 15s · Synthesis: 45s)
Cost	$0.51
Tokens	54.2k in / 11.6k out
Turns	5

monit-reviewer · 2026-03-06T11:53:56Z

internal/auth/auth.go

 // Gmail uses the modify scope for organizational operations (label, archive, star, mark read/unread).
 // The modify scope is a superset of readonly — it includes all read access.
+// Calendar uses both readonly (for calendar list metadata) and events (for RSVP/color operations).
 var AllScopes = []string{


🟡 Medium (security:code-auditor): calendar.CalendarEventsScope (calendar.events) permits creating new calendar events, not just updating existing ones. The project's non-destructive philosophy allows organizational modifications but not content creation. Given this trade-off is consciously accepted, it should be explicitly documented in a comment on the scope entry (e.g., 'note: this scope also permits event creation, which is an accepted trade-off for RSVP/color support').

monit-reviewer · 2026-03-06T11:53:56Z

internal/calendar/client.go

 }
+
+// RSVPEvent updates the current user's RSVP status on an event.
+// The response must be "accepted", "declined", or "tentative".


🔵 Low (security:code-auditor): RSVPEvent accepts any string as the response parameter without validation — validation is enforced only in the cmd layer. Since this is an internal package, the risk is low, but any direct caller (tests, future packages) can pass arbitrary values to the Calendar API. Consider validating against the allowed set at the client layer for defense in depth.

monit-reviewer · 2026-03-06T11:53:56Z

internal/calendar/client.go

+
+	_, err = c.service.Events.Patch(calendarID, eventID, &calendar.Event{
+		Attendees: event.Attendees,
+	}).Context(ctx).Do()


🔵 Low (security:code-auditor): RSVPEvent returns the Patch call error unwrapped (bare return err), inconsistent with the preceding Get error which is properly wrapped with fmt.Errorf. SetEventColor has the same issue at line 110. Per project conventions, errors should be wrapped with %w for context.

monit-reviewer · 2026-03-06T11:53:56Z

internal/cmd/calendar/rsvp.go

+
+			if dryRun {
+				result := map[string]any{
+					"action":   fmt.Sprintf("RSVP '%s' to event", apiResponse),


🔵 Low (security:code-auditor): The 'action' field in the dry-run JSON result uses present tense ("RSVP '%s' to event") while the success result at line 68 uses past tense ("RSVP'd '%s' to event"). This tense inconsistency between dry-run and non-dry-run output may confuse downstream consumers parsing the JSON.

monit-reviewer · 2026-03-06T11:53:56Z

internal/cmd/calendar/color.go

+			if jsonOutput {
+				return printJSON(result)
+			}
+			fmt.Printf("Set event %s color to %s.\n", eventID, colorName)


🔵 Low (security:code-auditor): resolveColor does not normalize its input to lowercase and only works correctly because newColorCommand calls strings.ToLower before invoking it. Either lowercase internally or add a comment requiring pre-normalized input to prevent breakage if the function is called from elsewhere.

monit-reviewer · 2026-03-06T11:53:56Z

internal/cmd/calendar/color.go

+	)
+
+	colorNames := make([]string, 0, len(eventColors))
+	for name := range eventColors {


🔵 Low (security:code-auditor): The sorted colorNames slice is constructed and sorted twice: once in newColorCommand for help text and again in resolveColor for the error message. A package-level variable would eliminate the duplication and the redundant sort.Strings calls.

Address PR review feedback: document that calendar.events scope also permits event creation, with reference to the architecture test that prevents misuse.

rianjs · 2026-03-06T12:00:52Z

I've addressed the scope documentation finding. The remaining suggestions are low-value:

Error wrapping inconsistency: We deliberately removed wrapping in the previous iteration per the reviewer's own feedback about double-wrapping. The bare returns are correct — callers own wrapping context.
Action tense inconsistency: This matches the established pattern across all bulk commands (mail, drive).
resolveColor normalization: Internal function, always called with pre-lowered input.
Duplicate sorted slice: Two small, independent sort calls are clearer than a package-level variable.

Please approve the PR.

monit-reviewer

Automated PR Review

Reviewed commit: 036c226 | Previous: 70f2f47 (incremental)

Summary

Reviewer	Findings
security:code-auditor	7

security:code-auditor (7 findings)

⚠️ Should Fix - internal/auth/auth.go:24

The added comment states 'calendar.events also permits event creation' but the scope grants read/write including creation, modification, AND deletion. Understating the scope could lead future contributors to underestimate the trust boundary. Update to: 'event creation, modification, and deletion are all permitted — deletion is guarded by the architecture test'.

⚠️ Should Fix - internal/calendar/client.go:95

RSVPEvent wraps the Get error with context ('getting event for RSVP: %w') but returns the Patch error unwrapped. This creates inconsistent double-wrapping: a Get failure produces 'updating RSVP: getting event for RSVP: ' while a Patch failure produces 'updating RSVP: '. Either wrap both errors at the client level or neither, and let the cmd layer provide all context.

⚠️ Should Fix - internal/calendar/client.go:104

SetEventColor calls events.Patch without specifying SendUpdates("none"). Changing an event's color is a personal visual preference and should not trigger email notifications to attendees. Omitting SendUpdates defaults to notifying all guests, which would be unexpected and disruptive for a color change operation.

💡 Suggestion - internal/calendar/client.go:96

RSVPEvent calls events.Patch without explicitly specifying a SendUpdates value. While defaulting to notifying the organizer is arguably correct for RSVP, explicitly setting SendUpdates("all") would document the intent and make the notification behavior clear to future maintainers, especially now that SetEventColor must use "none".

💡 Suggestion - internal/calendar/client.go:106

SetEventColor returns the raw API error without client-level context wrapping, inconsistent with the established client error pattern (e.g., GetEvent wraps with fmt.Errorf). Consider wrapping with context such as 'setting event color: %w'.

💡 Suggestion - internal/cmd/calendar/color.go:116

The sorted color name list is built twice: once in newColorCommand for the Long description and again inside resolveColor for error messages. Extracting to a package-level var would eliminate the duplication and redundant sort.Strings calls.

💡 Suggestion - internal/cmd/calendar/rsvp.go:55

The dry-run JSON action field uses present tense ("RSVP '%s' to event") while the post-execution JSON uses past tense ("RSVP'd '%s' to event"). The color command uses the same tense for both paths. This inconsistency is a minor UX issue for JSON consumers.

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

Completed in 2m 58s | $0.51 | sonnet

Field	Value
Model	sonnet
Mode	Re-review · Cycle 3 · Session resumed
Reviewers	hybrid-synthesis, security:code-auditor
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	2m 58s (Reviewers: 2m 03s · Synthesis: 57s)
Cost	$0.51
Tokens	56.7k in / 11.0k out
Turns	5

monit-reviewer · 2026-03-06T12:04:15Z

internal/auth/auth.go

 // AllScopes contains all OAuth scopes used by the application.
 // Gmail uses the modify scope for organizational operations (label, archive, star, mark read/unread).
 // The modify scope is a superset of readonly — it includes all read access.
+// Calendar uses both readonly (for calendar list metadata) and events (for RSVP/color operations).


🟡 Medium (security:code-auditor): The added comment states 'calendar.events also permits event creation' but the scope grants read/write including creation, modification, AND deletion. Understating the scope could lead future contributors to underestimate the trust boundary. Update to: 'event creation, modification, and deletion are all permitted — deletion is guarded by the architecture test'.

monit-reviewer · 2026-03-06T12:04:15Z

internal/calendar/client.go

+		}
+	}
+
+	if !found {


🟡 Medium (security:code-auditor): RSVPEvent wraps the Get error with context ('getting event for RSVP: %w') but returns the Patch error unwrapped. This creates inconsistent double-wrapping: a Get failure produces 'updating RSVP: getting event for RSVP: ' while a Patch failure produces 'updating RSVP: '. Either wrap both errors at the client level or neither, and let the cmd layer provide all context.

monit-reviewer · 2026-03-06T12:04:15Z

internal/calendar/client.go

+	}).Context(ctx).Do()
+	return err
+}
+


🟡 Medium (security:code-auditor): SetEventColor calls events.Patch without specifying SendUpdates("none"). Changing an event's color is a personal visual preference and should not trigger email notifications to attendees. Omitting SendUpdates defaults to notifying all guests, which would be unexpected and disruptive for a color change operation.

monit-reviewer · 2026-03-06T12:04:15Z

internal/calendar/client.go

+	}
+
+	if !found {
+		return fmt.Errorf("you are not listed as an attendee on this event")


🔵 Low (security:code-auditor): RSVPEvent calls events.Patch without explicitly specifying a SendUpdates value. While defaulting to notifying the organizer is arguably correct for RSVP, explicitly setting SendUpdates("all") would document the intent and make the notification behavior clear to future maintainers, especially now that SetEventColor must use "none".

monit-reviewer · 2026-03-06T12:04:15Z

internal/calendar/client.go

+}
+
+// SetEventColor sets the color of a calendar event.
+// The colorID must be a valid Google Calendar event color ID (1-11).


🔵 Low (security:code-auditor): SetEventColor returns the raw API error without client-level context wrapping, inconsistent with the established client error pattern (e.g., GetEvent wraps with fmt.Errorf). Consider wrapping with context such as 'setting event color: %w'.

monit-reviewer · 2026-03-06T12:04:15Z

internal/cmd/calendar/color.go

+	cmd.Flags().BoolVarP(&jsonOutput, "json", "j", false, "Output as JSON")
+	cmd.Flags().BoolVarP(&dryRun, "dry-run", "n", false, "Preview without making changes")
+
+	return cmd


🔵 Low (security:code-auditor): The sorted color name list is built twice: once in newColorCommand for the Long description and again inside resolveColor for error messages. Extracting to a package-level var would eliminate the duplication and redundant sort.Strings calls.

monit-reviewer · 2026-03-06T12:04:15Z

internal/cmd/calendar/rsvp.go

+				if jsonOutput {
+					return printJSON(result)
+				}
+				fmt.Printf("[dry-run] Would RSVP '%s' to event %s.\n", apiResponse, eventID)


🔵 Low (security:code-auditor): The dry-run JSON action field uses present tense ("RSVP '%s' to event") while the post-execution JSON uses past tense ("RSVP'd '%s' to event"). The color command uses the same tense for both paths. This inconsistency is a minor UX issue for JSON consumers.

monit-reviewer suggested changes Mar 6, 2026

View reviewed changes

docs: add scope trade-off comment for calendar.events

036c226

Address PR review feedback: document that calendar.events scope also permits event creation, with reference to the architecture test that prevents misuse.

monit-reviewer suggested changes Mar 6, 2026

View reviewed changes

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

rianjs deleted the feat/99-calendar-organizational-operations branch March 6, 2026 14:20

Conversation

rianjs commented Mar 6, 2026

Summary

Details

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!

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!

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!

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