Skip to content

feat(proto): add PullInDeadline, KpRotation, KpDeletion task variants + regen xmtp_proto#3805

Merged
insipx merged 1 commit into
mainfrom
insipx/taskrunner-recurrence-proto
Jul 6, 2026
Merged

feat(proto): add PullInDeadline, KpRotation, KpDeletion task variants + regen xmtp_proto#3805
insipx merged 1 commit into
mainfrom
insipx/taskrunner-recurrence-proto

Conversation

@insipx

@insipx insipx commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Proto-only base of the TaskRunner-recurrence stack. Adds three variants to the xmtp.mls.database.Task oneof (vendored regen):

  • PullInDeadline (tag 4) — generic one-shot "run a task sooner" primitive: lowers a target task row's next_attempt_at_ns to MIN(current, not_later_than_ns), targeting the row by its unique data_hash.
  • KpRotation (tag 5) / KpDeletion (tag 6) — recurring singleton tasks for key-package maintenance; empty payloads give each a stable data_hash so pull-ins can target them.

Minimal dispatch arms (warn + delete) keep xmtp_mls compiling standalone; real handlers land in the stacked PRs (#3806 generic layer, #3814 KP consumers).

gen/ is regenerated AHEAD of the upstream proto pin: xmtp/proto#339 carries all three messages; proto_version gets bumped to its merge SHA once it lands.

🤖 Generated with Claude Code

Note

Add PullInDeadline, KpRotation, and KpDeletion task variants to the MLS database worker

  • Adds three new Task oneof variants to the protobuf schema in xmtp.mls.database.rs: PullInDeadline (with target_data_hash and not_later_than_ns), KpRotation, and KpDeletion.
  • Regenerates serde impls and the proto descriptor binary to include these new types.
  • Adds placeholder match arms in tasks.rs that log a warning and delete the task — no business logic is implemented yet.

Macroscope summarized bd706d7.

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Claude finished @insipx's task —— View job


PR Review Complete

Review Date: 2026-07-06 16:20 UTC


Summary

This PR introduces proto-only scaffolding for TaskRunner recurrence features. The changes are minimal and well-scoped: three new task variants with stub handlers that log warnings and delete the tasks. This is appropriate preparatory work for the stacked PRs that will implement the actual handlers.

Findings

✅ No significant issues found

The implementation is sound for a proto-only PR:

  1. Stub handlers are appropriate (tasks.rs:315-334) - The placeholder arms correctly:

    • Log warnings with sufficient context (task ID, data hash for PullInDeadline)
    • Delete tasks to prevent accumulation
    • Include comments explaining real handlers land in stacked PRs
  2. Proto definitions are well-documented (xmtp.mls.database.rs:817-865):

    • PullInDeadline clearly documents its one-shot semantics and MIN operation
    • KpRotation and KpDeletion explain empty payloads yield stable data_hash
    • Type signatures are appropriate (bytes for hash, i64 for nanosecond timestamp)
  3. Generated code follows conventions - Serde impls and descriptor binary are auto-generated via standard tooling

  4. No security concerns - Stub handlers prevent any unintended behavior; real logic deferred to subsequent PRs

Minor Observations

  • Test coverage: Codecov reports 0% patch coverage for the 6 new lines in tasks.rs. Consider adding integration tests in stacked PRs that verify:

    • PullInDeadline correctly updates next_attempt_at_ns with MIN logic
    • KP tasks respect singleton semantics (single row per type via stable data_hash)
    • Pull-ins targeting non-existent tasks are idempotent
  • Error handling: The stub arms use ? propagation for delete_task (tasks.rs:324, 334). This is correct - DB errors should bubble up for retry logic.

Recommendation

Approved - This is clean foundational work. The placeholder handlers are safe and the proto definitions are clear. Proceed with the stacked PRs.


@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.69%. Comparing base (b86545d) to head (bd706d7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/xmtp_mls/src/worker/tasks.rs 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3805      +/-   ##
==========================================
- Coverage   84.70%   84.69%   -0.01%     
==========================================
  Files         409      409              
  Lines       60901    60907       +6     
==========================================
  Hits        51588    51588              
- Misses       9313     9319       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@insipx
insipx marked this pull request as ready for review July 2, 2026 16:39
@insipx
insipx requested a review from a team as a code owner July 2, 2026 16:39
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 2, 2026
@macroscopeapp

macroscopeapp Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Proto schema additions with stub handlers that only log and delete tasks. The comments note nothing creates these task types yet - this is preparatory scaffolding. All changes are in files owned by the author, and the bulk is auto-generated protobuf code.

No code changes detected at bd706d7. Prior analysis still applies.

You can customize Macroscope's approvability policy. Learn more.

@insipx insipx changed the title feat(proto): add PullInDeadline task variant + regen xmtp_proto feat(proto): add PullInDeadline, KpRotation, KpDeletion task variants + regen xmtp_proto Jul 2, 2026
@insipx
insipx force-pushed the insipx/taskrunner-recurrence-proto branch from 68b0215 to 3e7616c Compare July 2, 2026 18:20
@macroscopeapp
macroscopeapp Bot dismissed their stale review July 2, 2026 18:20

Dismissing prior approval to re-evaluate 3e7616c

… + regen xmtp_proto

PullInDeadline (tag 4): generic one-shot 'run a task sooner' primitive — lowers
a target task row's next_attempt_at_ns to MIN(current, not_later_than_ns).
KpRotation (tag 5) / KpDeletion (tag 6): recurring singleton tasks for key
package maintenance; empty payloads give each a stable data_hash for pull-ins.

Minimal dispatch arms keep xmtp_mls compiling standalone; real handlers land
in the stacked PRs. gen/ is regenerated AHEAD of the upstream proto pin —
xmtp/proto#339 carries all three messages; proto_version gets bumped to its
merge SHA once it lands.
@insipx
insipx force-pushed the insipx/taskrunner-recurrence-proto branch from 3e7616c to bd706d7 Compare July 6, 2026 16:19
@insipx
insipx merged commit 0d18307 into main Jul 6, 2026
44 checks passed
@insipx
insipx deleted the insipx/taskrunner-recurrence-proto branch July 6, 2026 21:16
insipx added a commit that referenced this pull request Jul 9, 2026
Stacked on #3806 (generic recurrence); the KpRotation/KpDeletion proto
variants land in #3805. Replaces the 5s-poll `KeyPackageCleaner` worker
with two recurring TaskRunner tasks.

## What

- **`KpRotation`** — recurring singleton: rotates + uploads a fresh key
package when the identity's rotation deadline
(`next_key_package_rotation_ns`) is due, then reschedules to the live
column (never a hardcoded +30d). On rotation it self-heals the deletion
singleton and pulls it in to the earliest pending `delete_at_ns`.
- **`KpDeletion`** — recurring singleton: sweeps expired local
key-package material, reschedules to the next pending deadline.
- **Welcome nudge** — after `queue_key_package_rotation` lowers the
column (~5s debounce, a security property), a durable `PullInDeadline`
pulls the rotation task in to match. Closes a real race: without it, a
seed that dispatched before the column write parks ~30d out and the
debounce is silently lost (reproduced; regression-tested).
- **Builder** seeds both singletons + reconciles deadlines to the live
DB columns on every startup (idempotent; also repairs rows stranded by a
crash between a column write and its pull-in).
- **Gutted** the old 5s-poll worker: loop/factory/rotate path deleted;
the struct + local-deletion helpers remain as the `KpDeletion` arm's
implementation.

## Behavior notes

- KP maintenance no longer has its own toggle — it is a critical MLS
function, deliberately coupled to the TaskRunner (no standalone
fallback). `WorkerKind::KeyPackageCleaner` remains in core and all three
bindings' enums for FFI compatibility, but enabling/disabling it is now
a no-op. Disabling the TaskRunner itself — per-kind via worker config or
globally via `disable_workers` — disables KP maintenance with it.
- Pre-registration (fresh client, no identity row): readers return
`None`/false, the rotation seed parks ~30d and converges silently once
registration writes the column. No transient errors.
- Zero migrations; zero reaper changes — recurring rows are
never-expiring seeds (`expires_at_ns = i64::MAX`, `max_attempts =
i32::MAX`), so the existing reaper predicate can't fire on them.

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- Macroscope's pull request summary starts here -->
<!-- Macroscope will only edit the content between these invisible
markers, and the markers themselves will not be visible in the GitHub
rendered markdown. -->
<!-- If you delete either of the start / end markers from your PR's
description, Macroscope will append its summary at the bottom of the
description. -->
> [!NOTE]
> ### Replace key-package cleaner worker with TaskRunner recurring tasks
for rotation and deletion
> - Removes the legacy `KeyPackagesCleanerWorker` and replaces it with
durable `KpRotation` and `KpDeletion` task rows managed by the
`TaskRunner` in
[key_package_maintenance.rs](https://github.com/xmtp/libxmtp/pull/3814/files#diff-1d4be0f0b8283620c52870358d20c933cc8086d19b8c804906ce1fc6d99af0eb).
> - `KpRotation` tasks reschedule to the DB-backed
`next_key_package_rotation_ns` deadline; `KpDeletion` tasks reschedule
to the earliest pending `delete_at_ns` across marked key packages.
> - `seed_and_reconcile_kp_tasks` is called at startup (after
`TaskWorker` registration) to seed both recurring tasks and pull in
deadlines from live DB state.
> - `queue_key_rotation` is now synchronous and atomically updates the
rotation column and enqueues a `PullInDeadline` nudge; `nudge_deletion`
does the same for deletion.
> - `is_identity_needs_rotation` now returns `false` pre-registration
and treats a `NULL` rotation column on an existing row as due
immediately, fixing a bug where migrated DBs with `NULL` deadlines were
skipped.
> - Behavioral Change: key-package maintenance no longer runs as a
standalone polling worker; it is now driven entirely by the `TaskRunner`
dispatch loop.
>
> <!-- Macroscope's changelog starts here -->
> #### Changes since #3814 opened
>
> - Modified key rotation queueing to insert-or-ignore rotation seed
tasks before enqueuing pull-in tasks [3cb7a61]
> - Added tests verifying rotation seed self-healing behavior during key
rotation nudges [3cb7a61]
> - Changed `KpDeletion` task rescheduling to use rotation interval
constant instead of fixed 30-day value [3cb7a61]
> - Updated mock implementation to match new trait signature [3cb7a61]
> <!-- Macroscope's changelog ends here -->
>
> <!-- Macroscope's review summary starts here -->
>
> <sup><a href="https://app.macroscope.com">Macroscope</a> summarized
74a0d28.</sup>
> <!-- Macroscope's review summary ends here -->
>
> <!-- macroscope-ui-refresh -->
<!-- Macroscope's pull request summary ends here -->

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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