feat: Implement support for journeys to sidetrack into other journeys#24
Merged
feat: Implement support for journeys to sidetrack into other journeys#24
Conversation
Threads TOutput through JourneyHandleRef, JourneyDefinition, defineJourney, defineJourneyHandle, TransitionResult.complete, and TerminalOutcome so a journey's terminal payload can be type-checked end-to-end. Existing call sites keep compiling — TOutput defaults to unknown. Adds the new authoring vocabulary without runtime behaviour: ChildOutcome (discriminated union for child terminal outcomes), InvokeSpec (the parent-side return shape for invoking a child journey by handle + named resume), and ResumeMap (sibling of TransitionMap keyed by [moduleId][entryName][resumeName]). ResumeMap lives at the JourneyDefinition level, not inside EntryTransitions: nesting an index-signature value in EntryTransitions' intersection collapses the mapped-type variance and breaks assignability to AnyJourneyDefinition. Keeping resumes separate sidesteps that and leaves resume names in their own keyspace, so a step can declare an exit and a resume that share a label without conflict. AnyJourneyDefinition now uses any for TModules instead of ModuleTypeMap so the structural variance check on ResumeMap admits any concrete TModules. No runtime work yet — runtime/outlet/persistence wiring lands in subsequent commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Core runtime change implementing the subroutine model: a parent journey's
transition can return `{ invoke: { handle, input, resume } }` to suspend
at the current step, run a child journey, and resume on a named handler
when the child terminates.
Runtime additions:
- InstanceRecord gains `parent` (back-link from child to parent) and
`activeChildId` (forward-link from parent to child). A reverse map
`childToParent` keeps the child-terminal hook O(1).
- `applyTransition` learns an invoke branch that validates the spec
(unknown journey, missing resume name, runtime.start failures) and
drives the parent into a discoverable abort on any failure path —
surfaced through the existing onError hook.
- After every transition, `applyResumeIfChild` checks whether the just-
transitioned record was a child whose terminal we just applied; if so
it looks up the parent's resume handler in `def.resumes[mod][entry]`,
fires it with `ChildOutcome` (typed payload on completed, reason on
aborted), and applies the result as the parent's next transition.
Recursion-safe: a parent that itself completes will trigger the
grandparent's resume, and so on up the call chain.
- `dispatchExit` blocks exits from a parent step that has a child in
flight — the parent advances only via the resume.
- `runtime.end` cascades: a parent that gets force-terminated takes its
in-flight child with it, with a propagated `parent-ended` reason so
child telemetry can distinguish the two paths.
Persistence (round-trips invoke state across reload):
- `SerializedJourney.pendingInvoke` (parent side) and `parentLink` (child
side) are emitted from `serialize()` when the parent has an in-flight
child whose record is in memory.
- `hydrateInto` restores both sides' link state. After every hydrate
path (sync start, async start, explicit hydrate), `relinkInvocations`
rebuilds `childToParent` from the in-memory pairs. Out-of-order hydrate
(child before parent or vice versa) ends up with a consistent map.
- A parent whose `activeChildId` references a not-yet-loaded child
stays in `active`; its step exits stay blocked. The shell decides
whether to load the missing child later or `runtime.end` to surrender
— auto-aborting would race with multi-step hydrates that legitimately
load parent and child in separate calls.
- `forget` / `forgetTerminal` clean up the reverse-map entry alongside
the keyIndex entry.
Public surface:
- `JourneyInstance` now exposes `activeChildId` and `parent` so the
outlet's call-stack walk and shells layering presentations can read
link state through the stable snapshot.
Tests cover: happy path (typed resume payload flows into parent state),
parent exits blocked while child is in flight, child abort surfaces as
ChildOutcome.aborted on the parent's resume, cascade-end on parent
termination, three-level nesting (outer→middle→leaf with bubbling
resumes), all four validation failure modes (unknown child journey,
unknown resume name, resume handler that throws, resume returns Promise),
and persistence round-trip in both forward and reverse hydrate order.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JourneyOutlet now walks the active call chain (root → activeChildId →
activeChildId → …) by default and renders the leaf step. A child journey
invoked from a parent step takes over the same outlet for the duration
of its run, matching the subroutine intuition.
leafOnly={false} opts out for layered presentations: the outlet stays on
the supplied instance's step even when a child is in flight, and a sibling
outlet keyed off `activeChildId` (or `useJourneyCallStack`) renders the
child elsewhere (modal, side-panel, etc.).
Internals:
- Subscribes to every instance along the chain so a link change anywhere
triggers a re-render. Bounded by MAX_CHAIN_DEPTH (64) with cycle
detection — defensive even though the runtime should not produce cycles.
- Step callbacks (`exit`, `goBack`) bind to the LEAF's record + reg so
exits dispatched by the rendered module hit the right journey. The
abandon-on-unmount cleanup targets the ROOT — `runtime.end` cascades
to the active child, so a single call cleans the whole chain.
- onFinished fires for the ROOT only. A child terminal triggers the
parent's resume; observers that want every termination subscribe via
registration-level onComplete/onAbort, not the outlet.
- onStepError's "abort" policy ends the LEAF (so the parent's resume sees
`outcome.aborted` and decides recovery), not the root.
Hook: useJourneyCallStack(runtime, rootId) returns the chain root-to-leaf.
Useful for shells that render parent and child outlets simultaneously
(parent under a modal, child inside it).
Tests cover: leaf-walk default rendering across invoke→resume hops,
leafOnly={false} stays on parent, onFinished does not fire on child
terminations but does fire on root completion, useJourneyCallStack
shifts as the chain grows and shrinks, abandon-on-unmount cascades to
the active child.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extend simulateJourney with two test modes for invoke/resume:
1. End-to-end: pass `children` in options to register reachable child
journey definitions; `sim.activeChild` returns a sub-simulator wrapped
around the runtime-registered child instance and drives it via the
same harness. Child terminations resume the parent automatically.
2. Mock the outcome: `sim.completeChild(payload)` synthesizes a
`{ status: "completed" }` outcome on the active child without
enumerating its transitions; `sim.abortChild(reason)` calls
`runtime.end` on the child. Useful when the child's path is
irrelevant to the parent-resume unit test.
Implementation:
- `JourneyRuntimeInternals.__synthesizeCompletion` is the runtime hook
the simulator calls to terminate a child via the standard
`applyTransition({ complete })` machinery — onComplete, onTransition,
persistence, and the parent's resume hook all fire as they would
for a real `{ complete }` transition.
- `wrapInstanceAsSim` is the shared constructor for both the primary
sim returned by simulateJourney and any child sim surfaced via
sim.activeChild. Same surface; recursive simulators for grandchildren
fall out naturally.
- A simulator built without `children` aborts on `invoke` with
`invoke-unknown-journey` — surfaces the missing registration loudly
rather than silently letting the parent stall.
Tests cover all four code paths (end-to-end success, end-to-end abort,
synthesized completion, runtime.end-driven abort) plus the throw-on-
no-child guard.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds discriminator and correlation fields to TransitionEvent so telemetry
consumers can filter to ordinary step transitions and skip the (often
noisy) invoke-bookkeeping events, or — when interested — read the child
identity on invoke and the typed outcome on resume.
- `kind: "step"` — ordinary `{ next | complete | abort }` (the default).
- `kind: "invoke"` — parent has just started a child journey. `child:
{ instanceId, journeyId }` is populated; `from` and `to` are equal
(parent's step doesn't change).
- `kind: "resume"` — parent's named resume handler has been applied with
the child's outcome. `outcome: ChildOutcome<unknown>` and `resume:
"<name>"` are populated; `from` / `to` reflect the actual transition
the resume handler returned.
Wiring threaded through `applyTransition`'s new optional `eventExtras`
parameter, defaulting to `kind: "step"` so existing call sites stay
on the step kind without modification. Kind events all flow through the
existing `onTransition` hook — no new subscription surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ADME Walks through the subroutine model end-to-end: the shape of an `invoke` transition + `resumes` map, why resume names are strings (persistence round-trip), the lifecycle/error matrix in table form, outlet leaf-walk behavior plus the layered-presentation pattern with useJourneyCallStack, the persistence blob shape, observability via TransitionEvent.kind, and the simulator's two test modes (real child sub-sim vs synthesized outcome). Adds a "What this is not" footer that explicitly calls out the design boundaries (no shared state, no concurrent spawn, no goBack across the boundary) so future authors don't try to bend the primitive into shapes it doesn't fit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Compact React Router example demonstrating the parent → invoke → child → resume flow end-to-end with persistence round-trip. Layout: - modules/checkout-review, modules/age-verify, modules/checkout-confirm — three plain modules that know nothing about journeys. - journeys/verify-identity — child journey, single-step age check that completes with a typed AgeVerificationToken (third generic on defineJourney pins TOutput). - journeys/checkout — parent journey that invokes the child mid-flow and reads `outcome.payload.token` typed end-to-end via the resume handler. The child's terminal aborts surface as `outcome.status === "aborted"` so the parent decides recovery. - shell — minimal RR app with one JourneyOutlet that follows the active leaf, plus useJourneyCallStack for a "Call stack" banner that visualizes the parent ↔ child relationship. Persistence: both journeys use createWebStoragePersistence. Reload mid-verification rehydrates parent + child from localStorage; the runtime relinks them via the in-memory `parent` / `activeChildId` fields. The verify modal pops right back up. Plugin signature update: - registerJourney now takes a fourth optional generic `TOutput = unknown` so journeys with a typed terminal payload (like the new verify-identity journey) keep typechecking through the plugin's registration surface. Default keeps existing call sites compiling. Lockfile regenerated (was corrupted by an earlier stash interaction). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same shape as the React Router variant — one parent journey (checkout) that invokes a child journey (verify-identity) mid-flow, picks up the typed terminal payload, and continues. Persistence round-trips both blobs so reload-during-verify relinks them automatically. Differences from the RR variant are scoped strictly to the routing surround: registry comes from @tanstack-react-modules/runtime, the shell uses TanStack's <Outlet>, the dev server runs on port 5176. Modules, journeys, persistence shape, and the demo flow itself are identical — same `<JourneyOutlet>` follows the leaf of the call chain by default; same `useJourneyCallStack` shows the parent → child relation in a banner. The two examples side-by-side are a useful regression check that the invoke / resume primitive is router-agnostic — the same journey definition exports work under both routing surfaces. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds invoke/resume support for journeys: parents can invoke typed child journeys, persist in-flight links, and resume with child terminal payloads. Core types, runtime, outlet, validation, simulator, tests, docs, and two complete examples (React Router and TanStack Router) are implemented or updated. Changes
Sequence DiagramsequenceDiagram
participant User
participant JourneyOutlet
participant JourneyRuntime
participant ParentJourney
participant ChildJourney
User->>JourneyOutlet: interact (e.g. "Proceed")
JourneyOutlet->>ParentJourney: dispatch exit
ParentJourney->>JourneyRuntime: evaluate transition → { invoke }
JourneyRuntime->>JourneyRuntime: validate & start child
JourneyRuntime->>ChildJourney: start(childHandle, input)
JourneyRuntime->>JourneyRuntime: set parent.activeChildId, child.parentLink, persist
JourneyOutlet->>JourneyRuntime: observe activeChildId
JourneyOutlet->>ChildJourney: render child leaf
User->>ChildJourney: complete (verified)
ChildJourney->>JourneyRuntime: child terminal (complete)
JourneyRuntime->>JourneyRuntime: apply parent's resume handler with child outcome
JourneyRuntime->>ParentJourney: perform resume transition
JourneyRuntime->>JourneyRuntime: clear links, persist
JourneyOutlet->>ParentJourney: render resumed parent step
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
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 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. Review rate limit: 0/1 reviews remaining, refill in 10 seconds.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: 3
🧹 Nitpick comments (5)
examples/react-router/journey-invoke/modules/age-verify/src/Verify.tsx (1)
36-46: 💤 Low valueMissing timeout cleanup on unmount.
If the component unmounts during the 250ms delay (e.g., user navigates away), the callback will still fire. While the journey runtime's step-token mechanism drops stale exits, cleaning up the timeout avoids unnecessary work and potential React warnings about state updates on unmounted components.
♻️ Proposed fix to clean up timeout
+import { useState, useRef, useEffect } from "react"; -import { useState } from "react"; import type { ModuleEntryProps } from "@modular-react/core"; import type { AgeVerifyExits } from "./exits.js"; export function Verify({ input, exit }: ModuleEntryProps<VerifyInput, AgeVerifyExits>) { const [confirming, setConfirming] = useState(false); + const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); + + useEffect(() => { + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + }; + }, []); // ... onClick={() => { setConfirming(true); - window.setTimeout(() => { + timeoutRef.current = window.setTimeout(() => { exit("verified", {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/journey-invoke/modules/age-verify/src/Verify.tsx` around lines 36 - 46, The onClick uses window.setTimeout without clearing it, so store the timeout id (from window.setTimeout) in a ref (e.g., timeoutRef) when you call it in the onClick handler and add a useEffect cleanup that clears that timeout on unmount (clearTimeout(timeoutRef.current)). Update references to the timeout id with the appropriate TypeScript type (number | null) if needed and ensure no further state updates occur from the timed callback after the timeout is cleared; keep using setConfirming and exit as-is inside the callback.packages/journeys/src/outlet-invoke.test.tsx (1)
306-324: ⚡ Quick winConsider adding a timeout to the Promise-based test.
The test wraps in a
Promiseand resolves viaqueueMicrotask, which is correct for testing the deferred abandon behavior. However, if the microtask never fires (due to a bug), the test would hang indefinitely.🛡️ Add a timeout to prevent hanging on failure
-describe("JourneyOutlet — abandon-on-unmount cascades to the active child", () => { - it("ends both the parent and the child when the outlet unmounts mid-invoke", () => - new Promise<void>((resolve) => { +describe("JourneyOutlet — abandon-on-unmount cascades to the active child", () => { + it("ends both the parent and the child when the outlet unmounts mid-invoke", { timeout: 1000 }, () => + new Promise<void>((resolve, reject) => { const rt = buildRuntime(); const id = rt.start(parentHandle, { orderId: "O-UM" }); const ui = render(createElement(JourneyOutlet, { runtime: rt, instanceId: id })); act(() => { ui.getByTestId("review-pick").click(); }); const childId = rt.getInstance(id)!.activeChildId!; ui.unmount(); // Abandon is deferred by a microtask to survive StrictMode mount/unmount. + const timeout = setTimeout(() => reject(new Error("Microtask did not fire")), 500); queueMicrotask(() => { + clearTimeout(timeout); expect(rt.getInstance(id)!.status).toBe("aborted"); expect(rt.getInstance(childId)!.status).toBe("aborted"); resolve(); }); })); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/journeys/src/outlet-invoke.test.tsx` around lines 306 - 324, The test "JourneyOutlet — abandon-on-unmount cascades to the active child" can hang if the microtask never runs; update the Promise in the test to include a timeout fallback: when creating the Promise in the it block, start a timer (e.g., setTimeout for ~1000ms) that fails the test (call reject or call expect(false).toBeTruthy() and resolve) and clear that timer when the queueMicrotask callback runs and calls resolve; reference the test's Promise, the rt/start pattern (rt.start(parentHandle,...)), the queueMicrotask callback, and the resolve call so you add the timer before mounting and clear it inside the microtask success path to ensure the test never hangs indefinitely.examples/react-router/journey-invoke/journeys/checkout/src/checkout.ts (1)
68-77: 💤 Low valueInline
new Date().toISOString()creates non-deterministic state.When
requiresAgeCheckis false, theverifiedAttimestamp is generated at transition time. This makes the journey state non-reproducible and could complicate testing or debugging. For a demo this is acceptable, but production code might prefer passing a clock or using a sentinel value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/journey-invoke/journeys/checkout/src/checkout.ts` around lines 68 - 77, The inline new Date().toISOString() in the transition to module "checkout-confirm" (entry "confirm") makes journey state non-deterministic; instead, pass a deterministic timestamp or sentinel from the journey context or state (e.g., a provided clock value like state.now / ctx.now) or use a stable sentinel/null for verifiedAt when requiresAgeCheck is false. Update the transition that builds verification ({ token: "skipped", verifiedAt: ... }) to consume an injected clock value or a precomputed state property rather than calling new Date() inline so the checkout flow (requiresAgeCheck, state.order, verification) remains reproducible and testable.examples/tanstack-router/journey-invoke/journeys/verify-identity/src/verify-identity.ts (1)
13-16: 💤 Low valueThe
attemptsfield is initialized but never used.The
VerifyIdentityStateincludes anattemptscounter initialized to0, but no transition increments or reads this value. This appears to be dead state that could be removed, or the retry logic may be missing.🧹 If retry logic is not needed, remove the unused field
export interface VerifyIdentityState { readonly customerId: string; - readonly attempts: number; } // ... - initialState: ({ customerId }: VerifyIdentityInput) => ({ customerId, attempts: 0 }), + initialState: ({ customerId }: VerifyIdentityInput) => ({ customerId }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/tanstack-router/journey-invoke/journeys/verify-identity/src/verify-identity.ts` around lines 13 - 16, The VerifyIdentityState declares an unused attempts field; either remove attempts from the VerifyIdentityState interface and any initial state construction/usages if no retry logic is needed, or implement retry handling: add a MAX_ATTEMPTS constant and update the attempts counter immutably in the verification flow (e.g., inside the verify/handleVerificationFailure/transition function that processes failed verification attempts) by incrementing state.attempts and gating retries when attempts >= MAX_ATTEMPTS; ensure you update all places that construct VerifyIdentityState (initializers) and any type references.packages/journeys/src/outlet.tsx (1)
413-421: 💤 Low valueCache key collision is theoretically possible but practically safe.
The cache key uses
fresh.join(">")which could collide if instance IDs contain the>character. For UUID-based IDs this is not a concern, but worth noting if custom ID schemes are ever supported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/journeys/src/outlet.tsx` around lines 413 - 421, The cache key in getStableSnapshot currently uses fresh.join(">") which can collide if InstanceId values include ">", so update getStableSnapshot to derive a collision-proof key (e.g., use a delimiter-safe encoding or serialize the array) before comparing/storing in cacheRef; update the logic around cacheRef, getStableSnapshot, and getSnapshot to use that new key so equality checks remain correct and avoid accidental collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/react-router/journey-invoke/shell/src/persistence.ts`:
- Around line 27-31: The comment describing the child namespacing is out of sync
with the key generation in keyFor: ({ input }) => `verify:${input.customerId}`;
update one of them to match the other — either change the comment to describe
that the key is `verify:${customerId}` or modify the keyFor implementation to
include the parent order id (for example
`verify:${input.orderId}:${input.customerId}` or
`verify:${parentOrderId}:${input.customerId}` depending on available fields) so
the code and comment are consistent; adjust any consumers or tests that expect
the previous format if you change the key format.
In `@examples/tanstack-router/journey-invoke/modules/age-verify/src/Verify.tsx`:
- Around line 33-55: The decline path can race with the scheduled
exit("verified", …): store the timeout id (e.g., timeoutRef via useRef) when
calling window.setTimeout in the onClick that calls setConfirming(true), and in
the decline onClick first guard against confirming (if (confirming) return),
disable the decline button by setting disabled={confirming}, and clear the
pending timeout via clearTimeout(timeoutRef.current) before calling
exit("declined", …); this ensures setConfirming, confirming, the scheduled
window.setTimeout callback, and exit are coordinated and only one exit is
emitted.
In `@packages/journeys/README.md`:
- Around line 1784-1785: The table's anchor links are broken because the
"Pattern - ..." headings generate anchors with three dashes (---) but the table
uses two (--); update the reference links for `selectModule` and
`selectModuleOrDefault` (and any other pattern links in the same table) to use
the triple-dash form matching the generated heading anchors (e.g., change
pattern--exhaustive-state-driven-module-dispatch-selectmodule to
pattern---exhaustive-state-driven-module-dispatch-selectmodule and
pattern--fallback-dispatch-selectmoduleordefault to
pattern---fallback-dispatch-selectmoduleordefault) so the fragments resolve
correctly. Ensure you update both table cells and run the markdown linter to
catch any remaining mismatches.
---
Nitpick comments:
In `@examples/react-router/journey-invoke/journeys/checkout/src/checkout.ts`:
- Around line 68-77: The inline new Date().toISOString() in the transition to
module "checkout-confirm" (entry "confirm") makes journey state
non-deterministic; instead, pass a deterministic timestamp or sentinel from the
journey context or state (e.g., a provided clock value like state.now / ctx.now)
or use a stable sentinel/null for verifiedAt when requiresAgeCheck is false.
Update the transition that builds verification ({ token: "skipped", verifiedAt:
... }) to consume an injected clock value or a precomputed state property rather
than calling new Date() inline so the checkout flow (requiresAgeCheck,
state.order, verification) remains reproducible and testable.
In `@examples/react-router/journey-invoke/modules/age-verify/src/Verify.tsx`:
- Around line 36-46: The onClick uses window.setTimeout without clearing it, so
store the timeout id (from window.setTimeout) in a ref (e.g., timeoutRef) when
you call it in the onClick handler and add a useEffect cleanup that clears that
timeout on unmount (clearTimeout(timeoutRef.current)). Update references to the
timeout id with the appropriate TypeScript type (number | null) if needed and
ensure no further state updates occur from the timed callback after the timeout
is cleared; keep using setConfirming and exit as-is inside the callback.
In
`@examples/tanstack-router/journey-invoke/journeys/verify-identity/src/verify-identity.ts`:
- Around line 13-16: The VerifyIdentityState declares an unused attempts field;
either remove attempts from the VerifyIdentityState interface and any initial
state construction/usages if no retry logic is needed, or implement retry
handling: add a MAX_ATTEMPTS constant and update the attempts counter immutably
in the verification flow (e.g., inside the
verify/handleVerificationFailure/transition function that processes failed
verification attempts) by incrementing state.attempts and gating retries when
attempts >= MAX_ATTEMPTS; ensure you update all places that construct
VerifyIdentityState (initializers) and any type references.
In `@packages/journeys/src/outlet-invoke.test.tsx`:
- Around line 306-324: The test "JourneyOutlet — abandon-on-unmount cascades to
the active child" can hang if the microtask never runs; update the Promise in
the test to include a timeout fallback: when creating the Promise in the it
block, start a timer (e.g., setTimeout for ~1000ms) that fails the test (call
reject or call expect(false).toBeTruthy() and resolve) and clear that timer when
the queueMicrotask callback runs and calls resolve; reference the test's
Promise, the rt/start pattern (rt.start(parentHandle,...)), the queueMicrotask
callback, and the resolve call so you add the timer before mounting and clear it
inside the microtask success path to ensure the test never hangs indefinitely.
In `@packages/journeys/src/outlet.tsx`:
- Around line 413-421: The cache key in getStableSnapshot currently uses
fresh.join(">") which can collide if InstanceId values include ">", so update
getStableSnapshot to derive a collision-proof key (e.g., use a delimiter-safe
encoding or serialize the array) before comparing/storing in cacheRef; update
the logic around cacheRef, getStableSnapshot, and getSnapshot to use that new
key so equality checks remain correct and avoid accidental collisions.
🪄 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: 5161d722-0413-43a1-b859-57a9f0ff92f9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (86)
examples/react-router/journey-invoke/README.mdexamples/react-router/journey-invoke/app-shared/package.jsonexamples/react-router/journey-invoke/app-shared/src/index.tsexamples/react-router/journey-invoke/app-shared/tsconfig.jsonexamples/react-router/journey-invoke/journeys/checkout/package.jsonexamples/react-router/journey-invoke/journeys/checkout/src/checkout.tsexamples/react-router/journey-invoke/journeys/checkout/src/index.tsexamples/react-router/journey-invoke/journeys/checkout/tsconfig.jsonexamples/react-router/journey-invoke/journeys/verify-identity/package.jsonexamples/react-router/journey-invoke/journeys/verify-identity/src/index.tsexamples/react-router/journey-invoke/journeys/verify-identity/src/verify-identity.tsexamples/react-router/journey-invoke/journeys/verify-identity/tsconfig.jsonexamples/react-router/journey-invoke/modules/age-verify/package.jsonexamples/react-router/journey-invoke/modules/age-verify/src/Verify.tsxexamples/react-router/journey-invoke/modules/age-verify/src/exits.tsexamples/react-router/journey-invoke/modules/age-verify/src/index.tsexamples/react-router/journey-invoke/modules/age-verify/tsconfig.jsonexamples/react-router/journey-invoke/modules/checkout-confirm/package.jsonexamples/react-router/journey-invoke/modules/checkout-confirm/src/Confirm.tsxexamples/react-router/journey-invoke/modules/checkout-confirm/src/exits.tsexamples/react-router/journey-invoke/modules/checkout-confirm/src/index.tsexamples/react-router/journey-invoke/modules/checkout-confirm/tsconfig.jsonexamples/react-router/journey-invoke/modules/checkout-review/package.jsonexamples/react-router/journey-invoke/modules/checkout-review/src/Review.tsxexamples/react-router/journey-invoke/modules/checkout-review/src/exits.tsexamples/react-router/journey-invoke/modules/checkout-review/src/index.tsexamples/react-router/journey-invoke/modules/checkout-review/tsconfig.jsonexamples/react-router/journey-invoke/shell/index.htmlexamples/react-router/journey-invoke/shell/package.jsonexamples/react-router/journey-invoke/shell/src/components/Home.tsxexamples/react-router/journey-invoke/shell/src/components/RootLayout.tsxexamples/react-router/journey-invoke/shell/src/main.tsxexamples/react-router/journey-invoke/shell/src/persistence.tsexamples/react-router/journey-invoke/shell/tsconfig.jsonexamples/react-router/journey-invoke/shell/vite.config.tsexamples/react-router/journey-invoke/tsconfig.base.jsonexamples/tanstack-router/journey-invoke/README.mdexamples/tanstack-router/journey-invoke/app-shared/package.jsonexamples/tanstack-router/journey-invoke/app-shared/src/index.tsexamples/tanstack-router/journey-invoke/app-shared/tsconfig.jsonexamples/tanstack-router/journey-invoke/journeys/checkout/package.jsonexamples/tanstack-router/journey-invoke/journeys/checkout/src/checkout.tsexamples/tanstack-router/journey-invoke/journeys/checkout/src/index.tsexamples/tanstack-router/journey-invoke/journeys/checkout/tsconfig.jsonexamples/tanstack-router/journey-invoke/journeys/verify-identity/package.jsonexamples/tanstack-router/journey-invoke/journeys/verify-identity/src/index.tsexamples/tanstack-router/journey-invoke/journeys/verify-identity/src/verify-identity.tsexamples/tanstack-router/journey-invoke/journeys/verify-identity/tsconfig.jsonexamples/tanstack-router/journey-invoke/modules/age-verify/package.jsonexamples/tanstack-router/journey-invoke/modules/age-verify/src/Verify.tsxexamples/tanstack-router/journey-invoke/modules/age-verify/src/exits.tsexamples/tanstack-router/journey-invoke/modules/age-verify/src/index.tsexamples/tanstack-router/journey-invoke/modules/age-verify/tsconfig.jsonexamples/tanstack-router/journey-invoke/modules/checkout-confirm/package.jsonexamples/tanstack-router/journey-invoke/modules/checkout-confirm/src/Confirm.tsxexamples/tanstack-router/journey-invoke/modules/checkout-confirm/src/exits.tsexamples/tanstack-router/journey-invoke/modules/checkout-confirm/src/index.tsexamples/tanstack-router/journey-invoke/modules/checkout-confirm/tsconfig.jsonexamples/tanstack-router/journey-invoke/modules/checkout-review/package.jsonexamples/tanstack-router/journey-invoke/modules/checkout-review/src/Review.tsxexamples/tanstack-router/journey-invoke/modules/checkout-review/src/exits.tsexamples/tanstack-router/journey-invoke/modules/checkout-review/src/index.tsexamples/tanstack-router/journey-invoke/modules/checkout-review/tsconfig.jsonexamples/tanstack-router/journey-invoke/shell/index.htmlexamples/tanstack-router/journey-invoke/shell/package.jsonexamples/tanstack-router/journey-invoke/shell/src/components/Home.tsxexamples/tanstack-router/journey-invoke/shell/src/main.tsxexamples/tanstack-router/journey-invoke/shell/src/persistence.tsexamples/tanstack-router/journey-invoke/shell/tsconfig.jsonexamples/tanstack-router/journey-invoke/shell/vite.config.tsexamples/tanstack-router/journey-invoke/tsconfig.base.jsonpackages/core/src/index.tspackages/core/src/journey-contracts.tspackages/journeys/README.mdpackages/journeys/src/define-journey.tspackages/journeys/src/handle.tspackages/journeys/src/index.tspackages/journeys/src/invoke.test.tspackages/journeys/src/outlet-invoke.test.tsxpackages/journeys/src/outlet.tsxpackages/journeys/src/plugin.tsxpackages/journeys/src/runtime.tspackages/journeys/src/simulate-journey-invoke.test.tspackages/journeys/src/simulate-journey.tspackages/journeys/src/types.tspackages/journeys/src/validation.ts
Fix the comment in the RR shell's verify-identity persistence to match the implementation (it was promising a parent-orderId-prefixed key but the keyFor only included customerId), and disable the decline button on both the RR and TSR age-verify modules while a verification timeout is pending — without it, a user could click "decline" during the 250ms window and emit two exits in a row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Composing journeys via invoke creates a directed call graph: A invokes
B, which may invoke C, possibly back into A. Without guards a cycle
becomes either an infinite chain that exhausts memory or a same-step
bouncing resume that pegs the CPU. This commit lands four layered
guards plus an opt-in static check at registration time.
* invokes[] declaration on JourneyDefinition — closed set of child
handles a journey may dispatch to. Strongly recommended; enables
the static cycle check and the runtime invoke-undeclared-child
guard for that journey.
* validateJourneyGraph (DFS over the invokes graph, deduping cycles
via canonical-rotation form) — runs as part of validateJourneyContracts
and rejects registrations that close a cycle, listing the path. Also
exported for shells composing registrations across plugin boundaries.
* invoke-cycle runtime guard — walks the active parent chain on every
invoke; if the target id is already on the stack the parent aborts
with the chain in the terminal payload.
* invoke-stack-overflow runtime guard — depth-cap with default 16,
resolved as min across non-undefined maxCallStackDepth on every
registration in the chain. Strictest journey wins; setting it to 1
blocks invoke from the journey outright.
* invoke-undeclared-child runtime guard — fires only when the parent
declared invokes[]; rejects dynamic dispatch to a handle missing
from the closed set.
* resume-bounce-limit runtime guard — caps consecutive same-step
resumes that return { invoke } instead of advancing. Counter resets
on any genuine step change and is persisted on the parent's blob
(resumeBouncesAtStep) so a reload-bounce sequence cannot reset the
budget through storage. Per-parent only.
All four runtime aborts surface through the existing onError channel
(phase: "invoke" or "resume") and produce structured terminalPayload
shapes that are safe to log. Tests cover each guard, mixed
configurations, the persistence round-trip, and the omitted-invokes[]
opt-out path.
Also fixes the broken anchor links in the API reference table flagged
by CodeRabbit (Pattern - … headings slug to three dashes, not two).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/journeys/src/runtime.ts (1)
1335-1359:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
goBack()bypasses the paused-parent guard and carries the old bounce counter onto a new step.This path should enforce the same
activeChildIdpause semantics asdispatchExit(). Right now a stalegoBackclosure can rewind the parent while a child is still in flight. It also never clearsresumeBouncesAtStep, so a later serialize/hydrate cycle can restamp an old bounce budget onto the new step and abort incorrectly.Suggested fix
function dispatchGoBack(record: InstanceRecord, reg: RegisteredJourney, stepToken: number) { if (record.status !== "active") return; + if (record.activeChildId) return; if (record.stepToken !== stepToken) return; if (record.history.length === 0) return; const step = record.step; if (!step) return; @@ record.hasRollbackSnapshot = record.rollbackSnapshots.some((s) => s !== undefined); record.step = previousStep; + record.resumeBouncesAtStep = null; record.stepToken += 1; record.updatedAt = nowIso(); record.cachedCallbacks = null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/journeys/src/runtime.ts` around lines 1335 - 1359, dispatchGoBack currently allows rewinding a parent while a child is paused and leaves resume bounce state intact; modify dispatchGoBack to mirror the paused-parent guard used in dispatchExit by returning early if record.activeChildId is set (or otherwise if the journey is paused by a child), and after performing the rollback set record.resumeBouncesAtStep = undefined and reset record.resumeBounces = 0 (or the equivalent fields used to track bounce budget) so the old bounce budget isn't carried onto the new step; keep the existing updates (step, stepToken, updatedAt, cachedCallbacks, persistence schedule, notify) but add the active-child guard near the top and clear the resume bounce fields right after applying the snapshot/setting the new step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/journeys/src/runtime.ts`:
- Around line 856-885: runtime.start can return an existing child; before
reassigning links, check if the returned child (from instances.get(childId))
already has child.parent set to a different parent id and if so treat it as "in
use": call applyTransitionLocal to abort the invoke (similar to the existing
"invoke-start-no-record" handling), optionally log via debug and call
fireOnError(parentReg, parent, /* err or message */, parentStep, "invoke"), and
then return false — otherwise proceed to set parent.activeChildId, child.parent
and childToParent as currently implemented.
- Around line 939-947: The code mutates linkage fields (child.parent = null and
parent.activeChildId = null) outside the standard transition/notify flow, which
leaves public snapshots returned by buildInstance()/getInstance() stale; change
the logic so clearance of invoke links is applied through the existing
transition/notify mechanism (or invoke the same notification step) instead of
direct mutation: move the parent/child unlinking into the same transition
function that emits updates (or call the instance update/notify function after
mutating) so that listeners get a new snapshot whenever child.parent or
parent.activeChildId is cleared; reference the child.parent and
parent.activeChildId mutations, the buildInstance()/getInstance() snapshot path,
and the transition/notify routine when implementing the fix.
---
Outside diff comments:
In `@packages/journeys/src/runtime.ts`:
- Around line 1335-1359: dispatchGoBack currently allows rewinding a parent
while a child is paused and leaves resume bounce state intact; modify
dispatchGoBack to mirror the paused-parent guard used in dispatchExit by
returning early if record.activeChildId is set (or otherwise if the journey is
paused by a child), and after performing the rollback set
record.resumeBouncesAtStep = undefined and reset record.resumeBounces = 0 (or
the equivalent fields used to track bounce budget) so the old bounce budget
isn't carried onto the new step; keep the existing updates (step, stepToken,
updatedAt, cachedCallbacks, persistence schedule, notify) but add the
active-child guard near the top and clear the resume bounce fields right after
applying the snapshot/setting the new step.
🪄 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: d395d700-60b5-4c14-86cc-ec7ad4e1048e
📒 Files selected for processing (11)
examples/react-router/journey-invoke/modules/age-verify/src/Verify.tsxexamples/react-router/journey-invoke/shell/src/persistence.tsexamples/tanstack-router/journey-invoke/modules/age-verify/src/Verify.tsxpackages/core/src/index.tspackages/core/src/journey-contracts.tspackages/journeys/README.mdpackages/journeys/src/index.tspackages/journeys/src/invoke-cycle-safety.test.tspackages/journeys/src/runtime.tspackages/journeys/src/types.tspackages/journeys/src/validation.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core/src/index.ts
- examples/react-router/journey-invoke/shell/src/persistence.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/react-router/journey-invoke/modules/age-verify/src/Verify.tsx
…broader test coverage
Addresses the post-implementation review of the cycle/recursion safety
work:
* `chain` payload on `invoke-cycle` aborts now matches the printed
warning — cycle portion only, pre-cycle prefix dropped. Telemetry
consumers parsing the payload and humans reading the log no longer
see different chains.
* `JourneySystemAbortReason` discriminated union and `isJourneySystemAbort`
predicate are exported from `@modular-react/core` (re-exported through
`@modular-react/journeys`). Authors and shells can narrow an `unknown`
abort payload to the runtime-emitted shapes via a single type guard;
author-supplied aborts whose `reason` field collides with a system
code are correctly excluded by checking against the closed code set.
* `resolveMaxCallStackDepth` no longer re-looks up the parent's
registration — call sites pass it directly.
* Documentation: README explicitly states that the cycle check is part
of the standard `validateJourneyContracts` pipeline; `0` / negative /
non-finite values for both new options are documented as "no opinion"
on the JSDoc and in the README; the abort-reason API table gains
`isJourneySystemAbort` with a worked narrowing example.
* New tests:
- `invokes: []` (empty set) rejects every dispatch.
- 3-level runtime cycle (A→B→C closing back to A) — the basic
case the original suite did not cover.
- 4-level runtime cycle where the cycle starts mid-chain (A→B→C→D
re-invoking B) — exercises the prefix-drop behavior in the
`chain` payload.
- Bounce-limit firing at the leaf of a multi-level chain bubbles
the abort up via the standard resume cascade; outer's terminal
payload reaches back to the original `resume-bounce-limit` reason.
- Depth cap can be lowered by the *child* journey, not only the
parent — verifies the documented "any journey in the chain can
lower" semantic.
- `0` / negative `maxCallStackDepth` is treated as "no opinion".
- `isJourneySystemAbort` narrows runtime-emitted payloads, rejects
author-supplied payloads with colliding-but-out-of-set `reason`
fields, and rejects null / undefined / non-objects.
* The previously-borderline "ignores edges to journeys not in the
registration set" test is paired with a runtime assertion proving
the missing-id failure mode (`invoke-unknown-journey`) still fires
even when `invokes` declared the missing handle.
203 tests pass (up from 195). Typecheck clean across the journey
package graph.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes the lint check on the PR — `oxfmt --check` was failing on the five files touched by the cycle-safety follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups on the cycle-safety PR: 1. Reject child theft on idempotent start. `runtime.start` returns the in-flight child id when a second invoker resolves to the same persistence key. The original code overwrote `child.parent` unconditionally, stealing the child from the first parent and stranding it behind a stale `activeChildId`. The second invoke now aborts with a typed `invoke-child-already-linked` reason carrying `childInstanceId` + `existingParentId`, and the original parent's link is preserved. 2. Refresh public snapshots on link clearance. `applyResumeIfChild` mutated `child.parent` and `parent.activeChildId` outside the normal transition/notify flow. Subscribers had already cached the pre-clear snapshot, so `getInstance(id)` kept returning the stale linkage. Notify the child after clearing its parent link, and notify the parent on the early-return arms (parent terminal, missing reg/step) that bail before `applyTransition` would have flushed it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- runtime.ts: dispatchGoBack now mirrors dispatchExit's paused-parent guard (drops when activeChildId is set) and clears resumeBouncesAtStep on rollback so a serialize/hydrate cycle can't restamp an old per-step bounce budget onto the new step. - outlet-invoke.test.tsx: add a 1s timeout fallback to the abandon-on-unmount Promise so a missing microtask fails fast instead of hanging the suite. - examples (RR + TSR) age-verify Verify.tsx: track the verification setTimeout in a ref and clear it on unmount. - examples (RR + TSR) verify-identity.ts: drop the unused \`attempts\` field from VerifyIdentityState/initialState. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@coderabbitai — addressed the remaining items from the two review summaries in 282c8f7. Fixed
Skipped (intentionally)
Tests + types still green ( |
|
Tip For best results, initiate chat on the files or code changes.
The rationale for the two intentional skips is also sound — the clock in the example communicates real intent to readers, and the 204 tests green is a great sign. The PR looks good to go! 🎉 If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
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
1 participant
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 by CodeRabbit
New Features
kindfield (step/invoke/resume) for clearer telemetry.Documentation
Tests