Conversation
Cross-cutting outcomes (cancelled, error, back, ...) tend to be emitted by many modules and handled the same way. wildcardTransitions lets a journey declare those handlers once instead of repeating them under every transitions[mod][entry] block. - New JourneyDefinition.wildcardTransitions with two precision tiers: byEntryAndExit[entry][exit] (module unknown, entry+exit known) and byExit[exit] (module + entry both unknown). Resolution is exact > byEntryAndExit > byExit; more specific always wins. - defineExitContract<T>(kind) shares an exit contract across modules. Wildcard handlers receive the contract's typed output instead of an intersection across modules. Optional StandardSchemaV1 (Zod / Valibot / ArkType / ...) drives runtime payload validation at every emit; failures abort with exit-payload-invalid, async schemas with exit-payload-invalid-async — both flow through isJourneySystemAbort. - validateJourneyContracts checks: live-key (some registered module emits the exit), contract identity consistency across modules, and warns on byEntryAndExit / byExit overlap. - 21 new tests (precedence, schema validation, validator rules, multi-step, observability) plus a TSD suite for type narrowing. - README pattern + API reference entries for wildcardTransitions and defineExitContract. https://claude.ai/code/session_01A2WdMdGoofyjzbjwzZJcMA
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds typed shared ExitContract (with optional synchronous StandardSchema validation), wildcard transition typing and registration-time/runtime validation, precedence-based handler resolution (exact → byEntryAndExit → byExit), package export updates, docs, and comprehensive tests. ChangesExit Contracts & Wildcard Transitions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/journeys/src/runtime.ts`:
- Around line 1354-1356: The current exit-contract validation silently no-ops
when moduleMap or options.modules are not provided: inside createJourneyRuntime,
detect when options.modules (and therefore moduleMap) are missing or when
moduleMap[step.moduleId] is undefined and the step references a moduleId, and
either log a clear warning or throw to fail-fast instead of silently skipping
validation; specifically update the block that reads stepModule =
moduleMap[step.moduleId], exitSchema = stepModule?.exitPoints?.[exitName] and
the isExitContract check to first assert moduleMap exists and stepModule is
found (or report the step.moduleId) and then proceed to evaluate exitSchema and
(ExitContract).schema so that validation cannot be bypassed when modules are
omitted.
In `@packages/journeys/src/validation.ts`:
- Around line 148-203: The code silently ignores malformed wildcard transition
containers; add explicit validation that pushes errors into the issues array
when wildcardTransitions is not an object, when
wildcardTransitions.byEntryAndExit exists but is not an object, and when
wildcardTransitions.byExit exists but is not an object (use the same error style
referencing def.id and the property path, e.g. `journey "${def.id}" has
malformed wildcardTransitions` / `...wildcardTransitions.byEntryAndExit` /
`...wildcardTransitions.byExit`), then only proceed with the existing
loops/logic when those values are objects; reference the existing symbols
wildcardTransitions, byEntryAndExit, byExit, modules, issues and keep existing
handler/function checks and validateContractConsistency calls.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ffc1532-8cee-4a95-a2f5-40ae8bd72373
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
packages/core/package.jsonpackages/core/src/entry-exit.tspackages/core/src/index.tspackages/core/src/journey-contracts.tspackages/core/src/types.tspackages/journeys/README.mdpackages/journeys/src/index.tspackages/journeys/src/runtime.tspackages/journeys/src/types.tspackages/journeys/src/validation.tspackages/journeys/src/wildcard-transitions.test-d.tspackages/journeys/src/wildcard-transitions.test.ts
- runtime.ts: warn (debug-only, deduped per moduleId) when dispatchExit
can't resolve the active step's module from the runtime's modules
map. Without this, ExitContract schema validation silently no-ops if
createJourneyRuntime is called without { modules: ... }.
- validation.ts: report malformed wildcardTransitions / byEntryAndExit
/ byExit (non-object values) as JourneyValidationError issues
instead of silently skipping them.
- Tests: cover both new behaviors (3 added).
https://claude.ai/code/session_01A2WdMdGoofyjzbjwzZJcMA
diogomiguel
reviewed
May 8, 2026
diogomiguel
reviewed
May 8, 2026
diogomiguel
approved these changes
May 8, 2026
Address the snapshot-staleness concern on validateJourneyContracts: the registration-time validator only sees modules registered with the registry at resolve time. Realistic dynamic flows (plugins, route- driven loading, HMR re-bundles, fresh test setups) re-trigger resolve() and therefore re-validate, but bypass paths (direct createJourneyRuntime use, plugin composition that adds modules post-validation, test mocks swapping descriptors) can leave drift uncaught. - Runtime now lazily walks its own moduleMap on the first dispatch of each contract-based exit and emits a dev-mode warning if two modules declare the same exit via different ExitContract instances. Cached per exit name (`spotCheckedExits` Set) so the cost is bounded — O(modules) once per unique exit fired, then amortized free. - Documented the lifecycle + spot-check in the README's contract pattern. - 2 new tests: drift triggers warn (and dedupes across instances); consistent contracts stay silent. https://claude.ai/code/session_01A2WdMdGoofyjzbjwzZJcMA
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/journeys/src/wildcard-transitions.test.ts (1)
765-773: ⚡ Quick winOrder-dependent regex may silently invert if the validator reorders its error accumulation.
The assertion:
.toThrowError( /malformed wildcardTransitions\.byEntryAndExit.*malformed wildcardTransitions\.byExit/s, );requires
byEntryAndExitto appear beforebyExitin the thrown message. IfvalidateJourneyContractsever processesbyExitfirst (alphabetically or by code change), the regex silently fails to match and the test regresses without a clear signal.Splitting into two independent assertions removes the ordering dependency:
🛡️ Proposed fix
- expect(() => - validateJourneyContracts( - [{ definition: def, options: undefined } as RegisteredJourney], - [profileModule, billingModule], - ), - ).toThrowError( - /malformed wildcardTransitions\.byEntryAndExit.*malformed wildcardTransitions\.byExit/s, - ); + let thrown: unknown; + try { + validateJourneyContracts( + [{ definition: def, options: undefined } as RegisteredJourney], + [profileModule, billingModule], + ); + } catch (e) { + thrown = e; + } + expect(thrown).toBeInstanceOf(JourneyValidationError); + expect(String((thrown as Error).message)).toMatch(/malformed wildcardTransitions\.byEntryAndExit/); + expect(String((thrown as Error).message)).toMatch(/malformed wildcardTransitions\.byExit/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/journeys/src/wildcard-transitions.test.ts` around lines 765 - 773, The test currently asserts the thrown message contains "malformed wildcardTransitions.byEntryAndExit" before "malformed wildcardTransitions.byExit" which makes it order-dependent; change the single ordered regex assertion into two independent assertions that each check for one substring so order doesn't matter: call expect(() => validateJourneyContracts([{ definition: def, options: undefined } as RegisteredJourney], [profileModule, billingModule])).toThrowError(/malformed wildcardTransitions\.byEntryAndExit/) and separately expect(...).toThrowError(/malformed wildcardTransitions\.byExit/), referencing validateJourneyContracts and the wildcardTransitions.byEntryAndExit / wildcardTransitions.byExit substrings to locate the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/journeys/src/wildcard-transitions.test.ts`:
- Around line 192-199: The test named "ignores the exit (warn) when no tier
matches" claims to verify a warning but uses freshRuntime with debug: false so
no warning is emitted; either rename the test to remove "(warn)" or add a new
test that creates the runtime with debug: true (via freshRuntime({ debug: true
}) or equivalent), spy on console.warn before calling rt.start/fireExit, assert
the spy was called with the expected "no matching tier" warning, and restore the
spy after the test; reference the existing test name and the
freshRuntime/rt.start/fireExit helpers to locate where to change or add the new
test.
---
Nitpick comments:
In `@packages/journeys/src/wildcard-transitions.test.ts`:
- Around line 765-773: The test currently asserts the thrown message contains
"malformed wildcardTransitions.byEntryAndExit" before "malformed
wildcardTransitions.byExit" which makes it order-dependent; change the single
ordered regex assertion into two independent assertions that each check for one
substring so order doesn't matter: call expect(() => validateJourneyContracts([{
definition: def, options: undefined } as RegisteredJourney], [profileModule,
billingModule])).toThrowError(/malformed wildcardTransitions\.byEntryAndExit/)
and separately expect(...).toThrowError(/malformed
wildcardTransitions\.byExit/), referencing validateJourneyContracts and the
wildcardTransitions.byEntryAndExit / wildcardTransitions.byExit substrings to
locate the check.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28fe829e-7d40-41db-ac47-cefd9300a49f
📒 Files selected for processing (4)
packages/journeys/README.mdpackages/journeys/src/runtime.tspackages/journeys/src/validation.tspackages/journeys/src/wildcard-transitions.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/journeys/src/validation.ts
- packages/journeys/README.md
- packages/journeys/src/runtime.ts
casamitjana
previously requested changes
May 8, 2026
casamitjana
reviewed
May 8, 2026
casamitjana
reviewed
May 8, 2026
casamitjana
reviewed
May 8, 2026
casamitjana
dismissed
their stale review
Changing this review to non-blocking. Follow-up comments are severity-labelled inline.
- runtime.ts: gate spotCheckContractDrift call site on debug flag so prod doesn't pay the function-call cost. Re-scope the spot-check to the dispatching journey's own modules (those keyed under def.transitions) — previously walked the runtime's full moduleMap, which could surface false positives from unrelated modules. Cache key is now [journeyId, exitName] so two journeys sharing an exit name each get their own first-fire check. - runtime.ts: attach a no-op .catch() to the dropped Promise from an async ExitContract schema. Without it, a rejecting thenable would surface as unhandledRejection after dispatchExit returns; we've already aborted the journey, the rejection value is irrelevant. - validation.ts: add isPlainObject guard — typeof === "object" alone admits null and arrays, both of which slipped through the earlier malformed-shape checks. Reject both with a descriptive "(got null|array|<typeof>)" suffix. - validation.ts: scope contract-consistency check to the journey's own declared modules (Object.keys(def.transitions)). The live-key check stays app-wide so typo-detection isn't weakened. Documented the asymmetric scopes in the new validateWildcardTransitions header. - validation.ts: extract validateWildcardTransitions / parseWildcardSlot / validateByEntryAndExit / validateByExit so the main registration loop reads cleanly. Each helper has a focused responsibility. - Tests: rename misleading "(warn)" test, add a sibling debug-warn test, add coverage for null/array shape guards, journey-scoped vs unrelated-module drift, and rejecting-async-schema unhandled- rejection suppression. 383 / 383 passing. https://claude.ai/code/session_01A2WdMdGoofyjzbjwzZJcMA
Round-2 review feedback shifted some semantics that the README described loosely: - The validator's two scope axes (live-key = app-wide, contract consistency = journey-scoped) are now spelled out as a per-check table. - The runtime spot-check description previously said "walks its own moduleMap" — wrong since e892472, which scoped the walk to the dispatching journey's own modules. Updated to "walks the dispatching journey's own modules (those keyed under def.transitions, intersected with the runtime's module map)" and clarified the debug-only gating ("production builds pay nothing"). - Added the plain-object shape rejection (rejects null and arrays) to the validation table. refactor(core): drop ExitContract.__contract sentinel `kind: string` is required on ExitContract and absent on plain ExitPointSchema (which defineExit() returns as `{}`), so the kind field already serves as the structural discriminator that isExitContract checks for. The extra __contract: true marker only defended against someone hand-constructing { kind: "x" } and passing it as an exit schema — an antipattern not worth optimizing for. Removing it gives a cleaner type with one fewer required field; the type predicate now keys off `typeof obj.kind === "string"`. 383 tests still pass. https://claude.ai/code/session_01A2WdMdGoofyjzbjwzZJcMA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
JourneyDefinition.wildcardTransitionsso cross-cutting outcomes (cancelled,error,back, ...) can be declared once instead of repeated under everytransitions[mod][entry]block, plusdefineExitContractfor cross-module shape consistency and optional Standard Schema runtime validation.Two wildcard tiers on
JourneyDefinition.wildcardTransitions:byEntryAndExit[entry][exit]— module unknown, entry + exit knownbyExit[exit]— module + entry both unknownResolution is exact →
byEntryAndExit→byExit, with the more specific handler always preempting the less specific. Encoded by lookup order indispatchExit— no flag, no merge step.defineExitContract<T>(kind, schema?)in@modular-react/core. When several modules reference the same contract value as the schema for an exit, the wildcard handler'soutputcollapses to the contract'sT(instead of the intersection across modules), andvalidateJourneyContractsenforces identity consistency at registration. The optional schema arg accepts anyStandardSchemaV1(Zod, Valibot, ArkType, ...) — payloads are validated at everyexit()emit; failures abort withexit-payload-invalid, async schemas withexit-payload-invalid-async. Both reasons flow through the existingisJourneySystemAbortpredicate.Validation at registration time covers: dead keys (no registered module emits the named exit / entry+exit pair), contract identity mismatch across modules sharing a wildcard slot, and a
console.warnwhen both tiers declare the same exit (the broader one is unreachable from the matching entry).Documentation: two new README pattern sections (
wildcard transitionsandshared exit contracts) plus API-reference entries fordefineExitContract/isExitContract/ExitContract.Test plan
nextadvance,onTransition, multi-step), plus 4 inwildcard-transitions.test-d.tsfor type narrowing.pnpm test.pnpm buildsucceeds.pnpm lintclean (zero new warnings; only 5 pre-existing in unrelated files).https://claude.ai/code/session_01A2WdMdGoofyjzbjwzZJcMA
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores