Skip to content

fix(types): prevent invalid dispatch metadata from bypassing Dialyzer - #73

Merged
yordis merged 2 commits into
mainfrom
yordis/fix-dispatch-option-types
Apr 14, 2026
Merged

yordis merged 2 commits into
mainfrom
yordis/fix-dispatch-option-types

Conversation

@yordis

@yordis yordis commented Apr 14, 2026

Copy link
Copy Markdown
Member
  • Tightens the public dispatch contract so invalid metadata shapes are visible to static analysis before they fail at runtime
  • Keeps the documented API expectation for dispatch metadata aligned with the exposed typespecs instead of hiding it behind a generic keyword list

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Apr 14, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Typespec-only changes that tighten the public dispatch/2 contract; low runtime risk but may surface new Dialyzer warnings or require downstream type adjustments for custom routers/apps.

Overview
Tightens the public dispatch/2 types across Commanded.Application, Commanded.Commands.Router, and Commanded.Commands.CompositeRouter by replacing the generic Keyword.t() with a new explicit dispatch_options()/Router.dispatch_options() union.

Expands dispatch_resp() to include {:ok, events :: [struct()]} and adds dedicated types for dispatch option keys/values (e.g., :metadata, :consistency, :returning, :timeout) so invalid option shapes are caught by static analysis instead of at runtime.

Reviewed by Cursor Bugbot for commit 5ce0842. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Apr 14, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6bf5c169-cef5-4be0-bcb8-f189ab94e2dd

📥 Commits

Reviewing files that changed from the base of the PR and between f05624e and 5ce0842.

📒 Files selected for processing (1)
  • lib/commanded/commands/router.ex

Walkthrough

Three Elixir modules updated to narrow type contracts for dispatch functions and introduce explicit dispatch option and return types in the Router; a successful dispatch may now be typed as {:ok, events :: [struct()]}.

Changes

Cohort / File(s) Summary
Application callback
lib/application.ex
Replaced `timeout_or_opts :: non_neg_integer()
Composite router
lib/commanded/commands/composite_router.ex
Updated @spec dispatch/2 to accept Router.dispatch_options() in place of Keyword.t() for the timeout_or_opts argument.
Router types & specs
lib/commanded/commands/router.ex
Added public types: dispatch_consistency/0, dispatch_return/0, dispatch_option/0, dispatch_options/0. Extended successful dispatch return to allow {:ok, events :: [struct()]}. Updated dispatch/2 callback to use dispatch_options().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through specs with nimble paws,

Replaced vague keywords with clearer laws.
Dispatch now lists what options mean,
Events return in a neat little scene —
Hooray for types! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: tightening dispatch type specifications to prevent invalid metadata from bypassing Dialyzer type checking.
Description check ✅ Passed The description clearly explains the rationale and intent of the changes, relating directly to the type specification updates in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yordis/fix-dispatch-option-types

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/commanded/commands/router.ex`:
- Around line 394-410: The public type dispatch_return() allows :events but
dispatch_resp() lacks a corresponding success shape, causing a type hole; update
dispatch_resp() to include the :events case (e.g., a variant representing
successful event lists) so Dialyzer knows the shape returned when {:returning,
:events} is used—locate the dispatch_return()/dispatch_resp() type definitions
and add an events-success variant (with an appropriate typed list element
matching your event type or map) to dispatch_resp() and any places that
pattern-match on its result.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a30177d5-8e74-4415-8653-c227cc3e7e5a

📥 Commits

Reviewing files that changed from the base of the PR and between fdaaa5d and f05624e.

📒 Files selected for processing (3)
  • lib/application.ex
  • lib/commanded/commands/composite_router.ex
  • lib/commanded/commands/router.ex

Comment thread lib/commanded/commands/router.ex Resolved
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis merged commit 495f886 into main Apr 14, 2026
5 checks passed
@yordis
yordis deleted the yordis/fix-dispatch-option-types branch April 14, 2026 19:05
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