Skip to content

fix: email-observation review fixes (pin skills, paginate Sent poll, dedup voice proposals)#1430

Merged
josephfung merged 5 commits into
cursor/email-observation-learning-e945from
fix/email-observation-review
Jul 16, 2026
Merged

fix: email-observation review fixes (pin skills, paginate Sent poll, dedup voice proposals)#1430
josephfung merged 5 commits into
cursor/email-observation-learning-e945from
fix/email-observation-review

Conversation

@josephfung

Copy link
Copy Markdown
Owner

Summary

Follow-up fixes for the three runtime deficiencies found reviewing #1419 / PR #1429. Targets the feature branch so it folds into that PR before merge. Part of the email-observation epic (#1419).

Every issue here only manifests at runtime and was missed by the original tests.

1. Skills weren't pinned to ceo-inbox — the flows couldn't run

Only ceo-inbox-sent-observe was pinned. The daily/weekly crons and the shadow-draft triage step call voice-learn, task-completion-from-sent, and ceo-inbox-shadow-draft too, but an unpinned skill is invisible to the agent. Pinned all three; documented the manual-enable requirement for the learning-digest companions in registry-defaults.yaml.

2. ceo-inbox-sent-observe dropped messages on busy days

listMessages({ limit: 20 }) returns only the newest 20 (Nylas is newest-first), so >20 sends between runs silently lost the older ones — lost voice diffs and completion candidates. Added CeoNylasClient.listAllMessages() (cursor pagination, maxScan ceiling) and switched the observer to it. On truncation the poll advances forward and logs accurately that older messages were skipped (a single newest-first floor can't drain a >ceiling backlog, and back-mining old history isn't a goal — realistically only a first-run/backfill event).

3. voice-learn re-proposed the same diffs every week

The pending-diffs doc is never consumed, so a still-above-threshold field appended a fresh ## Proposal — <field> block each run; the digest showed duplicates and the non-global status marker couldn't clear them all. Now reads existing proposals once per run and skips any field with an open (pending/approved) proposal; dismissed fields keep their cooldown path.

Also removed a dead regex in extractAskedTaskIds that never matched the written format.

Review notes

Both code-reviewer and silent-failure-hunter ran on these changes. They caught that my first truncation attempt (holding the watermark at minDate-1) was inverted — it would strand the older tail and re-scan forever. That's fixed in the final commit (advance forward + honest warning) with a regression test.

Test plan

  • New pagination test — follows next_cursor across pages, no page-1 drop
  • New truncation test — advances forward + warns, no held-watermark loop
  • New voice-learn dedup test — rerun doesn't duplicate an open proposal
  • pnpm run typecheck clean (all four projects)
  • 133 tests pass across all affected skills + shared helpers

Part of #1419

The daily sent-observe + weekly voice-learn crons and the shadow-draft
triage step reference these skills, but only ceo-inbox-sent-observe was
pinned. An unpinned skill is invisible to the agent, so voice-learn,
task-completion-from-sent, and ceo-inbox-shadow-draft could not be
invoked. Pin all three and document the manual-enable requirement for
the learning-digest companions in registry-defaults.

Signed-off-by: Joseph Fung <joseph@josephfung.ca>
listMessages returns only the newest 20 (Nylas is newest-first), so on a
day with >20 sends the older messages between the watermark and the newest
page were never observed — lost voice diffs and completion candidates.

Add CeoNylasClient.listAllMessages() that follows next_cursor up to a
maxScan ceiling, and switch the observer to it. When a run is truncated at
the ceiling, hold the watermark just below the oldest processed message so
the un-fetched tail is retried next run rather than stranded; re-processing
newer messages is safe because all writes are idempotent. Also drop a dead
second regex in extractAskedTaskIds that never matched the written format.

Signed-off-by: Joseph Fung <joseph@josephfung.ca>
The pending-diffs doc is never consumed, so a field that stays above
threshold re-derived a delta every weekly run and appended a fresh
'## Proposal — <field>' block. The digest then showed duplicates, and the
non-global status marker could only clear the first one on approve/dismiss.

Read the existing proposals once per run and skip any field that already
has an open (pending or approved) proposal; dismissed fields keep their
existing cooldown path. Add a regression test covering the rerun case.

Signed-off-by: Joseph Fung <joseph@josephfung.ca>
Signed-off-by: Joseph Fung <joseph@josephfung.ca>
…he tail

Review caught that the held-watermark logic was inverted: Nylas is
newest-first and received_after is an exclusive lower bound, so holding
the watermark at minDate-1 excludes the older un-fetched tail (date <
minDate) on every future poll while re-scanning the newest batch forever.
A single-floor poll cannot drain a >ceiling backlog and back-mining old
history isn't a goal, so on truncation just advance to the newest seen
and log accurately that older messages were skipped (not 'deferred').
Also harden listAllMessages so a final page overshooting a non-multiple
maxScan is still reported as truncated. Add a truncation regression test.

Signed-off-by: Joseph Fung <joseph@josephfung.ca>
@josephfung
josephfung merged commit 50d8fb2 into cursor/email-observation-learning-e945 Jul 16, 2026
2 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.

1 participant