Skip to content

feat: chat DM creation, calendar timezone, message filtering, find-group#146

Merged
omriariav merged 3 commits intomainfrom
feat/chat-calendar-fixes-142-145
Feb 24, 2026
Merged

feat: chat DM creation, calendar timezone, message filtering, find-group#146
omriariav merged 3 commits intomainfrom
feat/chat-calendar-fixes-142-145

Conversation

@omriariav
Copy link
Owner

Summary

Fixes four issues discovered during real usage:

Test plan

  • go build ./... compiles
  • go vet ./... clean
  • go test ./... all pass (including new tests for resolveIANA, parseTime, setup-space DM/GROUP_CHAT, after/before filter, spacecache Load/Save/FindByMembers, command registration)
  • Codex review passes

Closes #142, #143, #144, #145

🤖 Generated with Claude Code

… find-group (#142, #143, #144, #145)

- Fix calendar create/update sending "Local" as timezone — add resolveIANA() to resolve real IANA name (#143)
- Fix setup-space requiring --display-name for DMs/group chats — validate by type (#142)
- Add --after/--before convenience flags to chat messages for time filtering (#145)
- Add build-cache and find-group commands with persistent space-members cache (#144)
- Update chat skill docs to v2.2.0 with new commands and DM workflow recipe

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Summary
This PR adds timezone resolution for Calendar event creation/update, introduces Chat space-member caching with new chat build-cache and chat find-group commands, and adds --after/--before message filters plus setup-space validation tweaks.

What looks good

  • New Chat subcommands are registered and included in cmd/commands_test.go, matching the existing command structure patterns.
  • Docs for the chat skill and command reference were updated to cover new flags and commands.
  • Unit tests were added for parseTime, resolveIANA, and spacecache behaviors.

Issues found

Warning

  1. resolveIANA returns "" to “omit” timezone, but TimeZone is still assigned in Calendar requests. Unless calendar.EventDateTime.TimeZone is tagged omitempty, an empty string will be serialized and may be rejected by the API. Consider only setting TimeZone when non-empty.

    • cmd/calendar.go
  2. New commands (chat build-cache, chat find-group) lack httptest-based CLI behavior tests. The repository guidelines explicitly call for mock server tests for new commands. Only flag tests were added.

    • cmd/chat_test.go
    • cmd/commands_test.go
  3. README command table wasn’t updated for the new Chat commands and flags, while the instructions require it for new commands.

    • README.md (missing change)

Suggestion

  1. Validate --type for chat build-cache and chat setup-space to fail fast on invalid values instead of passing them through to the API. This prevents silent errors and aligns with tighter input validation.
    • cmd/chat.go

If you want, I can suggest specific test cases for the new Chat commands or draft a minimal README update.

Next steps (optional)

  1. Add httptest coverage for build-cache and find-group.
  2. Gate TimeZone assignment on non-empty resolve.
  3. Update README.md command table for the new flags and commands.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

I can’t compute the PR diff: origin/main...HEAD has no merge base in this repo, so Git can’t compare. How do you want to proceed?

Options:

  1. Provide the base branch name/commit SHA to diff against.
  2. Paste the diff here.
  3. Tell me to review git diff HEAD~N..HEAD for a specific N.

…ts, README

- Only set TimeZone when resolveIANA returns non-empty (avoids empty string serialization)
- Validate --type flag on setup-space and build-cache to fail fast on invalid values
- Add httptest mock server tests for build-cache and find-group flows
- Update README.md command table with new commands and flags
- Update CLAUDE.md chat command list

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Summary
This PR adds Google Chat member cache tooling (build-cache, find-group), adds --after/--before convenience filters for chat messages, and improves Calendar timezone handling for create/update, plus docs and tests updates.

What Looks Good

  • New chat subcommands are registered consistently and documented (cmd/chat.go, README.md, skills/chat/*).
  • chat setup-space now validates types and avoids displayName for DM/GROUP_CHAT, matching API behavior (cmd/chat.go).
  • resolveIANA and the related tests improve Calendar timezone correctness without breaking RFC3339 offsets (cmd/calendar.go, cmd/calendar_test.go).
  • commands_test.go includes the new chat subcommands, following the existing command-structure pattern (cmd/commands_test.go).

Issues Found

  1. Warning: Cache build silently populates spaces with zero members on membership list failures.
    In internal/spacecache/cache.go, member list errors break the paging loop but still write an entry with empty members, which can cause false negatives in find-group. Consider skipping the space entirely or returning an error for that space.
    File: internal/spacecache/cache.go

  2. Warning: find-group accepts empty/blank emails and can yield no matches unexpectedly.
    In cmd/chat.go, --members is split and trimmed but not filtered; a trailing comma or extra whitespace produces an empty needle, causing FindByMembers to fail matches. Recommend filtering out empty strings and erroring if the resulting list is empty.
    File: cmd/chat.go

  3. Warning: Missing command-level tests for build-cache and find-group flows.
    Current tests add flag checks and mock server list/members, but there are no tests that execute runChatBuildCache / runChatFindGroup end-to-end with a temp cache file and mocked Chat/People APIs. Given the project guideline (“new commands need httptest mocking and command structure tests”), coverage looks incomplete.
    Files: cmd/chat_test.go, internal/spacecache/cache_test.go

Open Questions / Assumptions

  • Should find-group return empty results (not an error) when the cache exists but contains zero spaces, or is the current “no cache found” error desired?
  • Should --after/--before validate RFC3339 locally for clearer error messages, or is letting the API reject invalid values acceptable?

Test Gaps / Notes

  • I did not run tests in this environment.

If you want, I can propose concrete test cases and a small patch for the two warnings above.

@omriariav omriariav merged commit 93687c5 into main Feb 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gws chat: fix DM creation and update skill docs

1 participant