feat: complete bug fix milestone with API, security, and DRY improvements#35
Open
Jarvis-AojDevStuio wants to merge 43 commits intomainfrom
Open
feat: complete bug fix milestone with API, security, and DRY improvements#35Jarvis-AojDevStuio wants to merge 43 commits intomainfrom
Jarvis-AojDevStuio wants to merge 43 commits intomainfrom
Conversation
- STACK.md - Technologies and dependencies - ARCHITECTURE.md - System design and patterns - STRUCTURE.md - Directory layout - CONVENTIONS.md - Code style and patterns - TESTING.md - Test structure - INTEGRATIONS.md - External services - CONCERNS.md - Technical debt and issues
- PROJECT.md - 28 issues across Gmail, Calendar, Drive, Forms modules - REQUIREMENTS.md - 19 traceable requirements (API, SEC, DRY, VAL, CACHE, CLEAN, DOC) - ROADMAP.md - 6 phases from API consistency to cleanup - STATE.md - Project state tracking - config.json - GSD workflow configuration Scope: All HIGH/MEDIUM/LOW issues from specs/bugs.md Approach: Clean break for API renames, tests + manual verification Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 01: API Consistency - Standard stack identified (TypeScript 5.6.2, Jest 29.7.0, ts-jest 29.1.2) - Architecture patterns documented (type-first refactoring, mock context testing) - Pitfalls catalogued (incomplete propagation, tool docs, test mismatches) - Code examples provided for parameter renames and unit tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 01: API Consistency - 2 plan(s) in 1 wave(s) - 2 parallel, 0 sequential - Ready for execution Plans: - 01-01: Gmail modifyLabels id parameter rename (API-01) - 01-02: Calendar EventResult eventId + deleteEvent type fix (API-02, API-03) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Changed EventResult.id to EventResult.eventId for consistency with input parameters - DeleteEventResult already correctly uses eventId field - EventSummary remains unchanged (intentional for list operations) - This change will be propagated to implementations in next commit
…eEventResult - read.ts: getEvent returns EventResult with eventId field - create.ts: createEvent and quickAdd return EventResult with eventId field - update.ts: updateEvent returns EventResult with eventId field - delete.ts: deleteEvent returns DeleteEventResult with eventId (removed success field) - All cache invalidation and logging updated to use eventId - Build passes with no TypeScript errors
- Update ModifyLabelsOptions.messageId to id - Update ModifyLabelsResult.messageId to id - Update modifyLabels implementation to use id throughout - Update JSDoc examples to use id parameter - Aligns with getMessage/getThread consistent id naming
- Add 8 comprehensive tests covering all modifyLabels scenarios - Test id parameter usage (not messageId) - Test result includes id field - Test cache invalidation with correct message id - Test logging with id parameter - Test performance tracking - Test add labels only, remove labels only, and both operations - All tests pass
- Created delete.test.ts with 11 comprehensive tests for deleteEvent - Tests verify DeleteEventResult return type with eventId and message - Tests verify no success field in result - Updated read.test.ts to check eventId instead of id in results - Updated cached result fixtures to use eventId field - All 70 calendar tests pass
- Change signature from messageId to id - Update example to use id parameter - Ensures AI agents see consistent parameter naming
Tasks completed: 3/3 - Task 1: Update Calendar types - EventResult and DeleteEventResult - Task 2: Update Calendar implementations - read, create, update, delete - Task 3: Create/update tests and documentation SUMMARY: .planning/phases/01-api-consistency/01-02-SUMMARY.md
Tasks completed: 3/3 - Renamed ModifyLabelsOptions/Result.messageId to id - Created 8 unit tests for modifyLabels - Updated tool documentation SUMMARY: .planning/phases/01-api-consistency/01-01-SUMMARY.md
- REQUIREMENTS.md: Mark API-01, API-02, API-03 as Complete - ROADMAP.md: Check off both plans (01-01, 01-02) as done - VERIFICATION.md: Add phase goal verification report (6/6 passed) Phase 1 achieved: Standardized parameter naming across Gmail and Calendar - Gmail: modifyLabels uses `id` parameter consistently - Calendar: EventResult returns `eventId` matching input - Calendar: deleteEvent returns DeleteEventResult with eventId Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 2: Security Fixes - Standard stack identified (Node.js built-ins, Jest testing) - Architecture patterns documented (query escaping, shared validation) - Pitfalls catalogued (incomplete escaping, validation bypass, over-escaping) - Code examples from official Google docs and existing codebase
Phase 02: Security Fixes - 2 plan(s) in 1 wave(s) - 2 parallel plans (both Wave 1) - Ready for execution Plans: - 02-01: Drive search query escaping (SEC-01) - 02-02: Gmail shared validation utilities (SEC-02) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add escapeQueryValue helper function with JSDoc - Escape single quotes in search query parameter - Escape single quotes in enhancedSearch query parameter - Escape single quotes in mimeType filter - Escape single quotes in parents filter - Prevents query injection via user input
- Extract sanitizeHeaderValue for CRLF injection prevention - Extract isValidEmailAddress for RFC 5322 validation - Extract encodeSubject for RFC 2047 non-ASCII encoding - Extract validateAndSanitizeRecipients for consistent validation - Extract encodeToBase64Url for Gmail API encoding - All functions have JSDoc comments and proper TypeScript types
- Add imports for validation functions from utils.js - Remove local function definitions (duplicates) - Use encodeToBase64Url helper for message encoding - buildEmailMessage stays in send.ts (Bcc handling differs from drafts) - All existing Gmail tests pass
- Create security test suite for query escaping - Test single quote escaping in search queries - Test multiple single quotes handling - Test query injection prevention - Test enhancedSearch query and filter escaping - Test combined query and filter escaping - 9 security tests covering all injection scenarios
- Import validation functions from utils.js - Validate email addresses in to, cc, bcc, from fields - Sanitize headers to prevent CRLF injection - Encode non-ASCII subjects with RFC 2047 - Use encodeToBase64Url helper for message encoding - Add security measures JSDoc comment to buildEmailMessage
Tasks completed: 2/2 - Add escapeQueryValue function and update search functions - Create Drive search security tests SUMMARY: .planning/phases/02-security-fixes/02-01-SUMMARY.md
utils.test.ts (26 tests): - sanitizeHeaderValue removes CR/LF characters - isValidEmailAddress validates RFC 5322 patterns - encodeSubject handles ASCII and non-ASCII subjects - validateAndSanitizeRecipients validates and sanitizes - encodeToBase64Url creates base64url encoding compose.test.ts (7 tests): - Validates email addresses in to, cc, bcc, from fields - Sanitizes CRLF in subject to prevent header injection - Creates drafts with valid inputs - Handles multiple valid recipients All 33 tests pass
Tasks completed: 4/4 - Create gmail/utils.ts with shared validation functions - Update send.ts to import from utils.ts - Add security validation to compose.ts - Create security tests for utils and compose SUMMARY: .planning/phases/02-security-fixes/02-02-SUMMARY.md
Phase 2 verified: all 7 must-haves passed, 42/42 tests passing Requirements completed: - SEC-01: Drive search escapes single quotes - SEC-02: Gmail compose.ts uses shared validation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 3: DRY Extraction - parseAttendees duplicated 3x across calendar files - buildEventResult duplicated ~440 lines across operations - encodeToBase64Url already extracted in Phase 2 (complete) - Extraction patterns verified from Phase 2 gmail/utils.ts - exactOptionalPropertyTypes compliance documented
Phase 03: DRY Extraction - 2 plan(s) in 2 wave(s) - Plan 01 (Wave 1): Create parseAttendees and buildEventResult in calendar/utils.ts with tests - Plan 02 (Wave 2): Update read.ts, create.ts, update.ts to use shared utilities - DRY-03 (encodeToBase64Url) already complete from Phase 2 - Ready for execution Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract canonical parseAttendees from create.ts - Type-safe transformation of Google Calendar API attendees - exactOptionalPropertyTypes compliance with explicit boolean checks - Handles email, displayName, responseStatus, organizer, self, optional fields
- Extract canonical buildEventResult from create.ts - Type-safe transformation of Google Calendar API event to EventResult - Calls parseAttendees internally for consistent attendee parsing - exactOptionalPropertyTypes compliance with conditional assignment - Handles all event fields: basic, creator, organizer, times, recurrence, attendees, conference, attachments, reminders
- 37 test cases covering validateEventTimes, parseAttendees, buildEventResult - Tests for edge cases: undefined, empty arrays, missing fields, all properties - Tests for type safety: null email handling, invalid responseStatus filtering - Tests for exactOptionalPropertyTypes: proper boolean handling - Tests for buildEventResult integration with parseAttendees - All tests passing with TypeScript strict mode
Tasks completed: 3/3 - Added parseAttendees utility to calendar/utils.ts - Added buildEventResult utility to calendar/utils.ts - Created comprehensive test suite (37 tests, all passing) SUMMARY: .planning/phases/03-dry-extraction/03-01-SUMMARY.md
- Import buildEventResult from utils.ts - Remove local parseAttendees function (35 lines) - Replace inline EventResult building (~113 lines) with buildEventResult call - Remove unused Attendee import - Total: ~148 lines removed Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Import buildEventResult from utils.ts - Remove local parseAttendees function (42 lines) - Replace inline EventResult building in createEvent (~113 lines) - Replace inline EventResult building in quickAdd (~113 lines) - Update logging to use result.attendees instead of parsedAttendees - Remove unused Attendee import - Total: ~268 lines removed Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Import buildEventResult from utils.ts - Remove local parseAttendees function (42 lines) - Replace inline EventResult building (~113 lines) - Remove unused Attendee import - Total: ~155 lines removed Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tasks completed: 3/3 - Update read.ts to use shared utilities - Update create.ts to use shared utilities - Update update.ts to use shared utilities Phase 3 complete - DRY extraction established 586 lines of duplicate code removed All 107 calendar tests passing SUMMARY: .planning/phases/03-dry-extraction/03-02-SUMMARY.md
- Phase 3 verified: 7/7 must-haves passed - DRY-01: parseAttendees exists only in calendar/utils.ts - DRY-02: buildEventResult exists only in calendar/utils.ts - DRY-03: encodeToBase64Url exists only in gmail/utils.ts (from Phase 2) - 586 lines of duplicate code eliminated - All 107 calendar tests passing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
All Phase 3 verification tests passed: - Build succeeds - 37/37 calendar utility tests pass - 107/107 calendar module tests pass - No duplicate parseAttendees implementations - buildEventResult properly used in all consumers Also updated CLAUDE.md to clarify that Claude should run commands directly instead of asking user to run them. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 04: Validation - Error behavior: throw descriptive, fail fast, standard Error - Validation strictness: all required fields, context-dependent arrays - modifyLabels: pre-validate labels, error on no-op/duplicates - Logging: error level, sanitized excerpts, redact PII
Phase 4: Validation - Standard stack identified (TypeScript native patterns) - Architecture patterns documented (type guards, assertion functions) - Pitfalls catalogued (null handling, PII logging, error messages) - Code examples with sources provided Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 04: Validation - 2 plan(s) in 1 wave(s) - Both parallel (Gmail and Calendar independent) - Ready for execution
Remove legacy bmad agent configs (16 agents across analysis, planning, research, review) and serena project state. Add GSD (Get Shit Done) framework with 11 agents, 27 commands, hooks, workflows, templates, and references. Move completed specs to archive. Add .beads/ to gitignore to exclude local daemon state. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove .beads/ local state tracking (replaced by GSD framework), clean up .gitignore, enable frontend-design plugin, and refactor CLAUDE.md into a concise table-driven format (~42% smaller). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 174 files, which is 24 over the limit of 150. You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Type Coverage ReportType Coverage: 97.56% This PR's TypeScript type coverage analysis is complete. |
Performance Comparison ReportOperation Performance
Memory Usage
Summary
Performance report generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive bug fix milestone delivering improvements across 4 phases with 43 commits, covering API consistency, security hardening, DRY extraction, and validation planning.
Phase 1: API Consistency
modifyLabelsparameter frommessageIdtoidfor consistencyeventIdinstead ofidwith newDeleteEventResulttypePhase 2: Security Fixes
utils.tswith shared validation functions (email validation, input sanitization)compose.tsandsend.tsto use centralized security validationPhase 3: DRY Extraction
parseAttendeesandbuildEventResultutilities intocalendar/utils.tscreate.ts,read.ts, andupdate.tsto use shared calendar utilitiesPhase 4: Validation (Planning)
Tooling & Infrastructure
.serena/and.beads/directoriesCLAUDE.mdproject documentation.planning/directory with codebase analysis and phase documentationType of Change
Test Plan
npm run buildto verify TypeScript compilation succeedsnpm testto verify all unit tests pass (including new tests for calendar utils, Drive search security, Gmail compose/utils, Gmail labels, and Calendar delete)eventIdparameter consistentlymodifyLabelsusesidparameterFiles Changed
src/modules/calendar/,src/modules/gmail/,src/modules/drive/,src/tools/utils.test.ts,delete.test.ts,search.test.ts,compose.test.ts,labels.test.tscalendar/utils.ts,gmail/utils.ts.planning/phase documentation and.claude/GSD frameworkChecklist
Co-Authored-By: AOJDevStudio