test(connectors): cover gmail + google-calendar pure-logic methods#89
Conversation
The session-start audit flagged @skytwin/connectors as having a single test (db-token-store) despite handling OAuth tokens and normalizing untrusted external data from Gmail and Google Calendar. Connectors feed raw signals into the decision pipeline, so silent normalization bugs cascade through every downstream layer. This pass focuses on pure-logic methods that don't need network mocking: gmail-connector (+20 tests): - Lifecycle: connect throws on missing token, succeeds with valid token, poll throws when not connected, disconnect clears state - inferEmailType (9 cases): newsletter (PROMOTIONS label, "newsletter" or "digest" subjects), subscription_renewal (subscription/renewal/ billing keywords), meeting_invite (meeting/invite/calendar keywords), grocery_reorder (order/delivery/grocery keywords), travel_alert (flight/hotel/travel/booking keywords), notification (noreply senders + UPDATES category), case-insensitive matching, work_email fallback - messageToSignal (6 cases): id+source format, case-insensitive header extraction, requiresResponse derivation for work/meeting/newsletter/ notification, internalDate ms → ISO timestamp, missing-headers safety google-calendar-connector (+15 tests): - Lifecycle: same shape as gmail - eventToSignal: id format, needsAction → meeting_invite + requiresResponse, accepted self-attendee → calendar_event, no-self-attendee fallback, conflict flag passthrough, all-day events - detectConflicts: empty, two-way overlap, back-to-back boundary (NOT a conflict), three-way overlap, all-day events ignored Net new tests: 35. connectors: 8 → 43 (+437%). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds unit coverage for the “pure-logic” portions of the Gmail and Google Calendar connectors in @skytwin/connectors, focusing on lifecycle guards (connected/disconnected) and deterministic normalization/classification logic that feeds the decision pipeline.
Changes:
- Adds lifecycle tests for
connect(),poll()precondition, anddisconnect()state reset for Gmail + Calendar connectors. - Adds classification/normalization tests for
GmailConnector(inferEmailType,messageToSignal) andGoogleCalendarConnector(eventToSignal,detectConflicts). - Introduces lightweight in-test OAuth token store stubs to avoid network mocking.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/connectors/src/tests/gmail-connector.test.ts | Adds lifecycle + inferEmailType + messageToSignal pure-logic test coverage. |
| packages/connectors/src/tests/google-calendar-connector.test.ts | Adds lifecycle + eventToSignal + detectConflicts pure-logic test coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import type { OAuthTokenStore } from '../oauth/token-store.js'; | ||
|
|
||
| function makeStubStore(token: { accessToken: string; refreshToken: string; expiresAt: Date } | null): OAuthTokenStore { | ||
| return { | ||
| save: async () => undefined, | ||
| get: async () => token, | ||
| delete: async () => undefined, | ||
| refreshIfExpired: async () => { | ||
| if (!token) { | ||
| throw new Error('No token stored'); | ||
| } | ||
| return token; | ||
| }, | ||
| } as unknown as OAuthTokenStore; |
There was a problem hiding this comment.
The stub token store doesn’t actually implement OAuthTokenStore (it uses save/get/delete instead of saveToken/getToken/deleteToken, and refreshIfExpired doesn’t accept userId/provider). Because it’s cast with as unknown as OAuthTokenStore, the test will still compile even if the connector starts using other OAuthTokenStore methods, but it could then fail at runtime or miss regressions. Update this stub to implement the real method names/signatures from oauth/token-store.ts and return an OAuthTokenSet-shaped object.
| import type { OAuthTokenStore } from '../oauth/token-store.js'; | |
| function makeStubStore(token: { accessToken: string; refreshToken: string; expiresAt: Date } | null): OAuthTokenStore { | |
| return { | |
| save: async () => undefined, | |
| get: async () => token, | |
| delete: async () => undefined, | |
| refreshIfExpired: async () => { | |
| if (!token) { | |
| throw new Error('No token stored'); | |
| } | |
| return token; | |
| }, | |
| } as unknown as OAuthTokenStore; | |
| import type { OAuthTokenSet, OAuthTokenStore } from '../oauth/token-store.js'; | |
| function makeStubStore(token: OAuthTokenSet | null): OAuthTokenStore { | |
| let storedToken = token; | |
| return { | |
| saveToken: async (_userId, _provider, nextToken) => { | |
| storedToken = nextToken; | |
| }, | |
| getToken: async (_userId, _provider) => storedToken, | |
| deleteToken: async (_userId, _provider) => { | |
| storedToken = null; | |
| }, | |
| refreshIfExpired: async (_userId, _provider) => { | |
| if (!storedToken) { | |
| throw new Error('No token stored'); | |
| } | |
| return storedToken; | |
| }, | |
| }; |
| it('disconnect() clears handler list and connection state', async () => { | ||
| const store = makeStubStore({ | ||
| accessToken: 'a', | ||
| refreshToken: 'r', | ||
| expiresAt: new Date(Date.now() + 60_000), | ||
| }); | ||
| const conn = new GmailConnector('user-1', store); | ||
| await conn.connect(); | ||
| conn.onSignal(() => {}); | ||
| await conn.disconnect(); | ||
| // After disconnect, poll should throw "not connected" again | ||
| await expect(conn.poll()).rejects.toThrow(/not connected/); | ||
| }); |
There was a problem hiding this comment.
This test name claims disconnect() clears the handler list, but the assertion only verifies that poll() throws after disconnect (i.e., connection state). Either assert the handler list is cleared (e.g., via the same cast-to-private approach used elsewhere) or rename the test to reflect what’s actually being verified.
| function makeStubStore(token: { accessToken: string; refreshToken: string; expiresAt: Date } | null): OAuthTokenStore { | ||
| return { | ||
| save: async () => undefined, | ||
| get: async () => token, | ||
| delete: async () => undefined, | ||
| refreshIfExpired: async () => { | ||
| if (!token) { | ||
| throw new Error('No token stored'); | ||
| } | ||
| return token; | ||
| }, | ||
| } as unknown as OAuthTokenStore; |
There was a problem hiding this comment.
The stub token store doesn’t actually implement OAuthTokenStore (it uses save/get/delete instead of saveToken/getToken/deleteToken, and refreshIfExpired doesn’t accept userId/provider). Because it’s cast with as unknown as OAuthTokenStore, the test will still compile even if GoogleCalendarConnector starts relying on other OAuthTokenStore methods, but it could then fail at runtime or miss regressions. Update this stub to implement the real method names/signatures from oauth/token-store.ts and return an OAuthTokenSet-shaped object.
| function makeStubStore(token: { accessToken: string; refreshToken: string; expiresAt: Date } | null): OAuthTokenStore { | |
| return { | |
| save: async () => undefined, | |
| get: async () => token, | |
| delete: async () => undefined, | |
| refreshIfExpired: async () => { | |
| if (!token) { | |
| throw new Error('No token stored'); | |
| } | |
| return token; | |
| }, | |
| } as unknown as OAuthTokenStore; | |
| type StubToken = NonNullable<Awaited<ReturnType<OAuthTokenStore['getToken']>>>; | |
| function makeStubStore(token: StubToken | null): OAuthTokenStore { | |
| return { | |
| saveToken: async (_userId, _provider, _token) => undefined, | |
| getToken: async (_userId, _provider) => token, | |
| deleteToken: async (_userId, _provider) => undefined, | |
| refreshIfExpired: async (_userId, _provider) => { | |
| if (!token) { | |
| throw new Error('No token stored'); | |
| } | |
| return token; | |
| }, | |
| } satisfies OAuthTokenStore; |
| it('disconnect() clears state and handlers', async () => { | ||
| const store = makeStubStore({ | ||
| accessToken: 'a', | ||
| refreshToken: 'r', | ||
| expiresAt: new Date(Date.now() + 60_000), | ||
| }); | ||
| const conn = new GoogleCalendarConnector('user-1', store); | ||
| await conn.connect(); | ||
| await conn.disconnect(); | ||
| await expect(conn.poll()).rejects.toThrow(/not connected/); | ||
| }); |
There was a problem hiding this comment.
This test name says disconnect() clears handlers, but it doesn’t register any handlers or assert anything about them—only that poll() throws after disconnect. Either add a handler and assert the internal handler list is cleared (if you’re comfortable testing internals here), or rename the test to only mention clearing connection state.
Captures the second half of the audit cleanup work that landed after #87: - #88 config/core test coverage (was 0/missing helpers) - #89 connectors gmail + calendar pure-logic tests (8 → 43) - #90 llm-client URL validation hardening (zone IDs, trailing dots, CGNAT) - #91 adapter-discovery factory shape check + manifest defaultConfig - #92 preference-archaeologist coverage (action keys, multi-group, expiry) - #93 worker SignalDeduper extraction + 11 tests Pure docs. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Closes the second-largest gap from the session-start audit: `@skytwin/connectors` had a single test (db-token-store) despite handling OAuth tokens and normalizing untrusted external data. Connectors feed raw signals into the decision pipeline, so silent bugs cascade.
This PR covers the pure-logic methods that don't need network mocking. Network-bound paths (poll → fetch, retry on 429) remain a follow-up — they need either a recorded fixture suite or a fetch mock layer.
`gmail-connector` — +20 tests
`google-calendar-connector` — +15 tests
Net coverage: connectors went 8 → 43 tests.
Test plan
🤖 Generated with Claude Code