Add hey event commands (list, create, edit, delete)#79
Add hey event commands (list, create, edit, delete)#79guilhermeyo wants to merge 28 commits intobasecamp:mainfrom
Conversation
The /calendars/:id/recordings endpoint rejects requests without starts_on/ends_on with 400. listPersonalRecordings already handles this via personalRecordingsLookback/Lookahead constants; apply the same window to the explicit --calendar path.
--calendar now accepts either a numeric ID or a case-insensitive owned calendar name. When a name can't be resolved (or is ambiguous), the CLI surfaces a helpful error pointing to 'hey calendars'. When no --calendar is given and the default personal calendar can't be determined, the error now lists owned calendars so the user can pick one.
When findPersonalCalendarID picks a calendar that HEY won't accept new events in (some accounts have an orphaned personal:true calendar), the raw 404 is unhelpful. Fall back to listing owned calendars and suggest --calendar explicitly — only triggered when --calendar was not set.
The Task 9 edit replaced the prior hint with --calendar info only; keep both since agents benefit from knowing the pipe-workflow too.
Aggregated cleanup from code review: - event.go: reuse default calendar list on 404 fallback, dedupe filter call, rename list-command local to match siblings, drop unreachable reminder nil guard - event_test.go: unify four runEvent* helpers into one, drop redundant server factories in favor of the Custom variants, use strconv.ParseInt, merge capturedRequest and capturedMethodPath
There was a problem hiding this comment.
Pull request overview
Adds a new hey event command group to manage calendar events (list/create/edit/delete) via the HEY SDK’s CalendarEvents service, plus supporting documentation and unit tests.
Changes:
- Introduces
hey event list|create|edit|deletecommands and calendar name/ID resolution. - Adds httptest-based unit coverage for event operations and default-calendar fallback behavior.
- Updates user-facing docs and command surface metadata to include the new commands.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/hey/SKILL.md | Documents the new hey event commands and examples for the skill surface. |
| internal/cmd/root.go | Registers the new event command on the root CLI. |
| internal/cmd/event.go | Implements hey event subcommands, reminder parsing, and calendar resolution. |
| internal/cmd/event_test.go | Adds unit tests covering list/create/edit/delete and calendar resolution behaviors. |
| README.md | Adds end-user documentation and examples for hey event. |
| API-COVERAGE.md | Records SDK endpoint coverage for event mutations. |
| .surface | Exposes the new commands/flags to the command surface list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="API-COVERAGE.md">
<violation number="1" location="API-COVERAGE.md:35">
P2: API coverage docs were updated for event create/edit/delete but omitted the new `hey event list` command mapping, causing documentation drift.</violation>
</file>
<file name="internal/cmd/event_test.go">
<violation number="1" location="internal/cmd/event_test.go:146">
P2: Tests miss coverage for the default-calendar POST 404 fallback path, so regressions in that branch would not be detected.</violation>
</file>
<file name="internal/cmd/event.go">
<violation number="1" location="internal/cmd/event.go:372">
P2: `event edit` allows an empty update request when no editable flags are provided, causing avoidable API calls/errors instead of a local usage error.</violation>
<violation number="2" location="internal/cmd/event.go:538">
P2: Default timezone logic can incorrectly force timed events to UTC when Go reports the local location as "Local", causing shifted event times.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Previously --timezone was silently ignored for all-day events, which is inconsistent with the flag help and can surprise users. Fail fast with a usage error instead.
Calling `hey event edit <id>` with no fields previously issued an empty PATCH request. Fail fast locally with a usage hint listing the editable flags.
time.Local.String() returns "Local" (not an IANA name) in some Go runtime configurations. Previously the code silently fell back to UTC, which would shift event times by the user's offset. Return an empty string from localTimezoneName in that case and require --timezone from the caller.
- README / SKILL: describe the real reminder format (non-negative number followed by m/h/d/w) instead of implying only the four sample values - API-COVERAGE: map 'hey event list' to the GetRecordings row alongside the other list-style consumers - event_test: cover the default-calendar-returns-404 fallback path so regressions in that error branch surface in CI
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
3 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/cmd/event_test.go">
<violation number="1" location="internal/cmd/event_test.go:347">
P2: The no-request assertion is weak: checking only empty body can pass even when a PATCH request was made with an empty payload.</violation>
</file>
<file name="internal/cmd/event.go">
<violation number="1" location="internal/cmd/event.go:190">
P2: When `--all-day` is set, `--start` and `--end` are silently accepted and ignored. This makes it easy to think a timed event was created when it's actually all-day. Add a validation check rejecting `--start`/`--end` when `--all-day` is present, similar to the `--timezone` check added here.</violation>
<violation number="2" location="internal/cmd/event.go:336">
P2: The edit command validates `--timezone` with `--all-day` but allows `--all-day` combined with `--start`/`--end`, which produces conflicting update parameters (all-day flag plus explicit start/end times). Add a similar rejection for `--start`/`--end` when `--all-day` is set.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Previously the flags were silently ignored when --all-day was also set, letting a user think they scheduled a timed event when they created an all-day one. Fail fast with a usage error, matching the treatment of --timezone + --all-day.
time.Local.String() returns "Local" on systems where the zone was loaded from /etc/localtime (the common case on Linux without $TZ set). Erroring out in that case forced users to always pass --timezone. Try $TZ next, then resolve /etc/localtime's symlink to recover the IANA name.
Previously only the request body was inspected, which would pass even for an empty PATCH. Check the captured method/path remain empty so the test actually verifies the early return.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/cmd/event.go">
<violation number="1" location="internal/cmd/event.go:588">
P2: Raw `$TZ` value is used as event timezone without IANA validation, so valid POSIX TZ formats can be sent to the API and fail.</violation>
</file>
<file name="internal/cmd/event_test.go">
<violation number="1" location="internal/cmd/event_test.go:225">
P2: Using `t.Skipf` on symlink setup failure skips the entire test, unintentionally dropping non-symlink assertions and reducing cross-platform coverage.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A review suggestion assigned []generated.CalendarEvent{} where
filterRecordingsByType returns []generated.Recording, breaking the
build. Use the correct type for the nil-to-empty slice fallback.
The CalendarEventsService landed in basecamp/hey-sdk@212ceb7 on main but has not been tagged. Using a pseudo-version pinned at that commit so CI can build this branch. Bump to v0.4.0 (or whatever the next SDK tag is) once it's released.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Defers run LIFO, so the previous order (server defer first, time.Local swap second) caused the time.Local restoration to run while the httptest server goroutines were still calling time.Now() — triggering -race. Swap the setup order so server.Close() runs before time.Local is restored.
- resolveCalendarID now rejects --calendar=0 or --calendar=-1 with a
clear usage error instead of falling through to a name lookup that
produces a misleading 'no owned calendar named "0"' message and an
unnecessary HTTP request.
- API-COVERAGE.md: drop the stray .json suffix on /calendars/{id}/
recordings; the SDK hits the path without it.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/cmd/event.go">
<violation number="1" location="internal/cmd/event.go:564">
P3: Whitespace-only `--calendar` values pass through to the name-lookup branch and produce a confusing error like `no owned calendar named ""`. Add an early check for `trimmed == ""` after `TrimSpace` and return a clear usage error (e.g., `--calendar cannot be empty`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…t IDs Pre-empt three adjacent gaps surfaced by the latest reviewer pass: - resolveCalendarID: reject empty/whitespace --calendar with a clear usage error instead of falling through to a name lookup that yields 'no owned calendar named ""'. - event edit: reject --title "" so an accidental blank doesn't silently wipe the event's summary on the server. - event edit/delete: reject non-positive event IDs locally instead of handing them to the API, mirroring what we already do for --calendar.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
time.LoadLocation("Local") succeeds (Go treats it as an alias for the
process-local zone), so the previous check let the literal string
"Local" through to the HEY API where it isn't a meaningful IANA name.
Reject it explicitly with a usage hint pointing at real IANA names.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refs #48.
Adds
hey event list|create|edit|deleteon top of theCalendarEventsServiceadded to the SDK in basecamp/hey-sdk#30 — thanks @monorkin for the SDK piece, this plugs into it with no further SDK changes.Commands
--calendaraccepts a numeric ID or the name of an owned calendar (case-insensitive).--reminderaccepts30m,1h,2d,1wand is repeatable.Scope intentionally matches
CreateCalendarEventParams/UpdateCalendarEventParamsexactly — title, date, start/end or all-day, timezone, reminders. Location, description, attendees, recurrence, and countdown aren't wired up here; they'd need corresponding SDK additions first.Default calendar UX
When
--calendaris omitted the CLI uses the existingfindPersonalCalendarIDhelper (same onehey recordingsuses). On some accounts thepersonal: truecalendar returns 404 on event creation — when that happens we catch the 404 and print the owned calendar list with a--calendar <id-or-name>hint instead of a bare HTTP error. Same list is shown iffindPersonalCalendarIDcan't find a personal calendar at all.Test plan
Summary by cubic
Adds CLI support to list, create, edit, and delete calendar events with
hey event. Improves timezone detection and input validation, with clearer errors when choosing calendars.New Features
hey event list|create|edit|delete--calendar <id-or-name>,--limit,--all,--ids-only(uses a safe date window when targeting a specific calendar)--title,--date,--all-dayor--start/--end; optional--timezone(defaults to local for timed events; resolves via$TZand/etc/localtime, requires--timezoneif undeterminable; rejects"Local"); repeatable--reminder(m|h|d|w, rejects overflow);--timezoneand--start/--endcannot be combined with--all-day; trims whitespace--title,--date,--start,--end,--all-day,--timezone,--reminder; requires at least one flag; rejects empty--title;--timezoneand--start/--endcannot be combined with--all-day; trims whitespacehey event delete <id>(rejects non‑positive IDs)--calendaraccepts an ID or case-insensitive owned name; rejects empty/whitespace and non-positive IDs; helpful errors for unknown or ambiguous names with a hint to runhey calendars--calendaris omitted; if missing or event creation returns 404, shows owned calendars and suggests--calendar-raceDependencies
github.com/basecamp/hey-sdk/gotov0.3.1-0.20260407122900-212ceb7d1fe6to consumeCalendarEventsuntil the next SDK tag is released.Written for commit 82cfd59. Summary will update on new commits.