Skip to content

fix(connector-meet-fathom): restore cursor to latest end_time#36

Merged
sabbah13 merged 4 commits into
mainfrom
fix/fathom-cursor-regression
May 5, 2026
Merged

fix(connector-meet-fathom): restore cursor to latest end_time#36
sabbah13 merged 4 commits into
mainfrom
fix/fathom-cursor-regression

Conversation

@sabbah13

@sabbah13 sabbah13 commented May 5, 2026

Copy link
Copy Markdown
Owner

Summary

Two-commit fix to make main CI green again:

  1. fix(connector-meet-fathom) — initialise maxEnd = '' so a meeting with end_time older than since can still advance the cursor. The existing uses end_time as cursor test has been failing on main since it was added.
  2. ci — run pnpm build before typecheck/lint/test. Without this, packages that depend on @cowork-tasks/core can't resolve it during typecheck (the same fix is bundled into PR feat(0.4.14): VSCode-kanban feature parity (restore_task, rename_label, undo, column-aware new task, TASKS_DIR) #35).

Together these unblock main's CI, which has been red for the last 5 consecutive runs.

Test plan

  • pnpm --filter @cowork-tasks/connector-meet-fathom test -- 2/2 pass locally
  • Full workspace build green
  • All 82 artifact e2e green

Cowork Tasks Maintainers and others added 2 commits May 4, 2026 22:23
…time, not since

Initialising maxEnd to `since` meant the cursor could never move to a
meeting whose end_time was older than since (e.g. in tests that supply
a fixed past timestamp). Initialize to '' and fall back to since only
when no meetings are returned, so the cursor always reflects the latest
end_time seen.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

sabbah13 commented May 5, 2026

Copy link
Copy Markdown
Owner Author

Automated Code Review

Verdict: ✅ APPROVE

Summary

Two-commit CI unblock: moves pnpm build before typecheck/lint/test in the CI pipeline so cross-package type resolution works correctly in the monorepo, and fixes the Fathom connector's cursor logic by initialising maxEnd = '' instead of maxEnd = since, then returning maxEnd || since to safely fall back when no meetings are returned. The fix is minimal, targeted, and the test plan confirms both connector tests and the full workspace build pass.


Issues (must fix before merge)

None.


Suggestions (optional)

  1. end_time empty-string edge case [packages/connector-meet-fathom/src/index.ts]

    The return maxEnd || since fallback correctly handles an empty meetings array, but if any FathomMeeting.end_time can be an empty string (e.g. an in-progress meeting that hasn't ended yet), that meeting would win the > '' comparison and set maxEnd = '' — which the || since fallback would then silently overwrite. Worth checking whether FathomMeeting.end_time is typed as string or string | undefined. If the type allows empty/absent values, a guard like if (m.end_time && m.end_time > maxEnd) would be safer.

  2. Cursor regression if end_time predates since [packages/connector-meet-fathom/src/index.ts]

    The fix intentionally allows meetings with end_time < since to advance maxEnd. If the Fathom API's since parameter filters on a different field (e.g. start_time or created_at), all returned meetings could have end_time < since, causing the cursor to move backward on every poll and potentially triggering duplicate task creation. This may be the accepted trade-off for fixing the failing test, but a short comment on the let maxEnd = '' line explaining the invariant (e.g. // since filters by start_time; end_time may predate it) would protect future readers from re-introducing the original initialisation.


This review was generated automatically. A maintainer will follow up.


Generated by Claude Code

Cowork Tasks Maintainers and others added 2 commits May 4, 2026 23:15
Pre-existing strict-mode issues surfaced when CI started running build
before typecheck. Fixes:

- App.tsx: groupBy='source' patch needs the source-type cast against
  NonNullable<Task['source']> (not Task['source'] which is nullable).
- App.tsx: deriveColumns now returns Column-shaped objects with an
  always-present color (defaulted to #6b6a64).
- TopBar.tsx: drop the unused _onRefresh prop entirely; explicit
  comment notes Cowork chrome owns reload.
- App.tsx: drop the JSX comment that crept in on the TopBar opening.
- Markdown.tsx: remove unused IMAGE_EXT constant.
- useTasks.ts: replace eslint-disable for react-hooks/exhaustive-deps
  with a real comment (the rule isn't installed and was failing as
  unknown). The closure-over-apply behavior is intentional.
- server.ts: replace require('node:fs')/require('node:path') with
  named imports so eslint's no-var-requires passes.
- journey.spec.ts: drop unused setupCoworkEnv import.
- storage.test.ts: cast globalThis through unknown for stricter TS.
- connector-meet-fathom: cursor advancement was clamped to "now-24h"
  on first run because maxEnd started at `since`. Now starts at the
  original cursor (or empty) so any returned meeting wins; falls back
  to since only when there were no meetings.

10/10 mcp + 15/15 storage + 84/84 e2e + 2/2 fathom + lint + typecheck
all green locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sabbah13
sabbah13 merged commit 5582c7a into main May 5, 2026
2 checks passed
@sabbah13
sabbah13 deleted the fix/fathom-cursor-regression branch May 5, 2026 17:29
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.

1 participant