enforce runAt and runAfter exclusivity in EnqueueOptions

[reeceyang]

It looks like runAt and runAfter fields in EnqueueOptions are meant to be exclusive, but this isn't forced by the types currently.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Summary by CodeRabbit

• Bug Fixes
  • Improved type safety for task scheduling options: the compiler now enforces that runAt and runAfter cannot be specified together, preventing conflicting scheduling choices. This is a compile-time-only change with no runtime behavior impact.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4acdfaf3-a080-4b4e-ac65-675de8d3f1b4

📥 Commits

Reviewing files that changed from the base of the PR and between d440d8b and f7ce2ba.

📒 Files selected for processing (1)

• src/client/index.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/client/index.ts

---

📝 Walkthrough

Walkthrough

Enhanced the EnqueueOptions TypeScript union to enforce mutual exclusivity between runAt and runAfter by adding runAfter?: never to the runAt branch and runAt?: never to the runAfter branch. No runtime behavior changes.

Changes

Cohort / File(s)	Summary
Type Constraint Enhancement src/client/index.ts	Updated EnqueueOptions union: added runAfter?: never to the branch with runAt?: number and added runAt?: never to the branch with runAfter?: number, enforcing compile-time mutual exclusivity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐇 I hopped through types both near and far,
runAt and runAfter now sit apart,
With never I nudge them to choose one way,
No runtime ripples, just compile-time play,
A small tweak, a tidy day. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: enforcing mutual exclusivity between runAt and runAfter fields in EnqueueOptions through TypeScript type modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch reece/enforce-runAt-and-runAfter-exclusivity

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/workpool/@convex-dev/workpool@178

commit: f7ce2ba

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/client/index.ts (1)

443-456: ⚠️ Potential issue | 🟡 Minor

Fix formatting inconsistency to pass Prettier check.

The mutual exclusivity pattern using never is correct and idiomatic TypeScript. However, there's inconsistent punctuation: line 445 uses a semicolon (runAfter?: never;) while line 455 uses a comma (runAt?: never,). This inconsistency is likely contributing to the Prettier failure flagged in CI.

Run prettier --write to auto-fix, or manually ensure consistent punctuation across both branches.

Suggested fix

       runAt?: number;
-
-      runAfter?: never;
+      runAfter?: never;
     }
   | {
       /**
        * The number of milliseconds to run the action after.
        * If not provided, the action will be run as soon as possible.
        * Note: this is advisory only. It may run later.
        */
       runAfter?: number;
-
-      runAt?: never,
+      runAt?: never;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/client/index.ts` around lines 443 - 456, The union branches that define
runAt/runAfter are inconsistent: in the first branch the property "runAfter?:
never;" ends with a semicolon, while in the second branch "runAt?: never," ends
with a comma, causing Prettier failure; edit the second branch to use the same
punctuation (change the trailing comma on the "runAt?: never" line to a
semicolon) or run prettier --write to normalize formatting in the type
definition that contains the runAt/runAfter mutually exclusive properties.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/client/index.ts`:
- Around line 443-456: The union branches that define runAt/runAfter are
inconsistent: in the first branch the property "runAfter?: never;" ends with a
semicolon, while in the second branch "runAt?: never," ends with a comma,
causing Prettier failure; edit the second branch to use the same punctuation
(change the trailing comma on the "runAt?: never" line to a semicolon) or run
prettier --write to normalize formatting in the type definition that contains
the runAt/runAfter mutually exclusive properties.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0d451fc0-4855-4eff-a8d7-67a333dd6b4d

📥 Commits

Reviewing files that changed from the base of the PR and between d494518 and d440d8b.

📒 Files selected for processing (1)

• src/client/index.ts

[ianmacartney]

I think most of the time it'll warn if you're providing a key that is unexpected, but if you have an object with both as optional it wouldn't complain. Can you check if the Workflow component uses this type or will be impacted by this?

[reeceyang]

It looks like workflow is going to be impacted since it redefines its own version of its type to pass into the workpool enqueue function. I can change it there as well

[ianmacartney]

sounds good - let's update workflow first before releasing this version. though folks shouldn't see issues if they have mismatched versions unless they have skipLibCheck: false I suspect?

[ianmacartney]

I released a version of workflow that also makes this change