feat(journeys,core): add runtime.goForward + JourneyInstance.future redo stack

[diogomiguel]

Summary

Adds a redo / forward primitive to the journey runtime: runtime.goForward(id) and JourneyInstance.future. Inverse of goBack, so shells can wire browser Forward (or an in-page redo button) to actual journey navigation instead of having the URL drift ahead of the runtime.

Motivation

goBack was destructive — it popped history + rollbackSnapshots and threw the data away. A shell wiring browser Forward had no way to redo the rewind because the original exit name + output had flowed from the step component into the runtime, not back out. Cases like this:

exit(stepA) → state at B
browser Back → URL /A + runtime back at A
browser Forward → URL /B but runtime stuck at A

…left the URL out of sync with the rendered step until the user re-submitted the form. Browser semantics expect Forward to replay the rewind; the runtime now supports that.

What changed

InstanceRecord.future: a stack of { step, state, snapshot } entries. dispatchGoBack pushes the step it rewinds from before mutating; dispatchGoForward inverts the operation — pops the top, pushes the current step back onto history, restores state verbatim, re-attaches the snapshot to rollbackSnapshots so a subsequent goBack rewinds correctly.

goForward does NOT re-run the original transition handler. The future entry holds the post-transition state directly; redo restores it without invoking side effects (API calls, telemetry, persistence in the transition body would double-fire on every Forward press). If a shell needs business logic to re-execute, it fires the exit itself instead.

Forward-stack invalidation mirrors the browser: applyTransition clears future on any next / complete / abort arm. invoke leaves it intact (parent step doesn't advance while a child runs).

Public surface:

• runtime.goForward(id): void on JourneyRuntime
• instance.future: readonly JourneyStep[] on JourneyInstance (lets shells gate a Forward button on instance.future.length > 0)
• ModuleEntryProps.goForward?: () => void — closure form for steps that surface an in-page redo control (most shells will wire Forward at the shell level instead)
• simulator.goForward() and harness.goForward(id) for authoring + testing
• JourneyOutlet passes goForward through alongside goBack

Transient: not persisted, reset on hydrate, reset when a record is recycled. Reload starts empty, same as opening a new browser tab.

Test plan

• pnpm test — 411 tests passing (399 prior + 12 new runtime-go-forward tests covering single goBack→goForward, state restoration, multi-step rewind+redo, no-op when empty / unknown / terminal, future invalidation on fresh exit, history symmetry, instance.future snapshot exposure, active-child no-op, rollback-mode redo discarding intermediate edits, subscriber notify)
• pnpm typecheck clean
• pnpm build clean across all packages
• Validated end-to-end via downstream consumer (autopilot's <JourneyHost> calling useJourneyUrl wired to runtime.goForward) — browser Back/Forward round-trips work for a real wizard flow

Notes

• No breaking changes to existing API. goBack keeps its destructive semantics for callers that don't care about redo (just record.future.push(...) is added before the existing pop).
• bindStepCallbacks adds a canGoForward gate based on record.future.length > 0 — the new closure is undefined when there's no redo target, matching the goBack pattern.
• Internal JourneyRuntimeInternals.__bindStepCallbacks return type now includes goForward?: () => void (test harness uses this).

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added redo ("go forward") so users can restore the last rewound step and its captured state after using "go back".
  • Step-level controls now optionally receive a redo callback to surface forward/redo actions.
  • Simulator and test harness expose a goForward control to mirror runtime behavior.

• Tests

  • Added comprehensive tests covering redo/forward behavior, edge cases, persistence, and interaction with rollback and child journeys.

[coderabbitai]

Warning

Rate limit exceeded

@diogomiguel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 16 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1d0fdd53-4f6e-40fd-b826-7615dfd5f4e3

📥 Commits

Reviewing files that changed from the base of the PR and between 9abc542 and 7903c10.

📒 Files selected for processing (10)

• CHANGELOG.md
• packages/core/src/journey-contracts.ts
• packages/core/src/types.ts
• packages/journeys/README.md
• packages/journeys/src/outlet.tsx
• packages/journeys/src/runtime-go-forward.test.ts
• packages/journeys/src/runtime.ts
• packages/journeys/src/simulate-journey.test.ts
• packages/journeys/src/simulate-journey.ts
• packages/journeys/src/testing.ts

📝 Walkthrough

Walkthrough

Adds a transient in-memory redo stack and a runtime API goForward(id) that restores the last rewound step and state (without re-running transition handlers), wires a goForward callback to step components, exposes simulator and test-harness methods, and provides comprehensive tests validating redo semantics.

Changes

Redo (goForward) Navigation

Layer / File(s)	Summary
Public API contracts for redo packages/core/src/journey-contracts.ts, packages/core/src/types.ts	JourneyInstance adds future (unpersisted redo stack), JourneyRuntime adds goForward(id) method, and ModuleEntryProps adds optional goForward callback for redo controls.
Runtime core implementation packages/journeys/src/runtime.ts	Introduces FutureEntry, adds InstanceRecord.future, pushes redo targets during dispatchGoBack, implements dispatchGoForward to restore history/rollback/state/step without re-running original handlers, clears future on forward-invalidating transitions, updates lifecycle/hydration to reset future, and exposes goForward(id) on the runtime surface.
UI component wiring packages/journeys/src/outlet.tsx	JourneyOutlet binds and passes goForward callback to rendered step components alongside exit and goBack.
Simulator and test harness packages/journeys/src/simulate-journey.ts, packages/journeys/src/testing.ts	JourneySimulator exposes goForward() delegating to runtime; JourneyTestHarness adds goForward(id) with instance validation and guidance when redo is unavailable.
Comprehensive test suite packages/journeys/src/runtime-go-forward.test.ts, packages/journeys/src/simulate-journey.test.ts	New tests validate redo behavior across scenarios: single/multiple rewinds, state restoration, no-op conditions (empty future, unknown/loading/terminal instances), forward-invalidation semantics, child-journey safety, rollback-mode correctness, subscriber notifications, and persistence correctness.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• kibertoad/modular-react#38: Extends JourneyRuntime navigation behavior by adding the redo (goForward) capability paired with earlier goBack dispatch changes.

Suggested reviewers

• kibertoad

Poem

🐰 Hop back, then forward — the journey's in store,

A future of steps kept in memory's drawer,
Rewind then restore, no handler replayed,
The path is preserved where the redo is laid,
I munch on a carrot and celebrate more.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main changes: adding runtime.goForward() method and JourneyInstance.future redo stack, which are the core features introduced in this pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch journeys/go-forward

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 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/simulate-journey.ts`:
- Around line 310-312: The simulator's goForward method calls
harness.goForward(instanceId) which throws when redo is unavailable, but the
simulator interface (and runtime.goForward) should no-op instead; modify
goForward to catch or check for redo availability and return without throwing
when no redo exists (e.g., wrap the harness.goForward(instanceId) call in a
conditional or try/catch and silently return when the harness indicates no-op),
preserving existing behavior when redo is available and still delegating to
harness.goForward for the actual redo.

🪄 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: 6194d04f-edc3-48ce-a62b-99acca48bef8

📥 Commits

Reviewing files that changed from the base of the PR and between a97bfd7 and 8c882d3.

📒 Files selected for processing (7)

• packages/core/src/journey-contracts.ts
• packages/core/src/types.ts
• packages/journeys/src/outlet.tsx
• packages/journeys/src/runtime-go-forward.test.ts
• packages/journeys/src/runtime.ts
• packages/journeys/src/simulate-journey.ts
• packages/journeys/src/testing.ts

[kibertoad]

Code review — runtime.goForward + JourneyInstance.future

Verified locally: pnpm install && pnpm -w build && pnpm --filter @modular-react/journeys test → 413/413 passing, no type errors.

The change is clean, symmetric with dispatchGoBack, and the test suite is one of the most thorough I've seen for a feature this size. State/step/history/rollbackSnapshots/hasRollbackSnapshot are all restored consistently — I traced rollback-mode and preserve-state flows step by step and the invariants hold (the captured state is the post-transition value, step.input was already built against that state, and the popped snapshot lines back up with the right history index on redo).

Nothing blocking. A few things worth considering:

1. Asymmetric simulator behavior — goBack throws, goForward no-ops

packages/journeys/src/simulate-journey.ts:307-316

simulator.goBack() delegates to harness.goBack() (throws on empty history). simulator.goForward() deliberately bypasses the harness wrapper and calls runtime.goForward() directly so it's a silent no-op. The inline comment explains the intent, but the public-facing JSDoc on the interface (goForward(): void) doesn't surface this asymmetry to authors writing simulator-driven tests — a misordered goBack in a test will fail loudly, a misordered goForward will silently mis-assert downstream. Consider either:

• Routing simulator.goForward through the harness too (consistent loud failure), or
• Adding a sentence to the JSDoc making the no-op-vs-throw asymmetry explicit so test authors understand they need to assert state, not rely on the throw.

2. TransitionEvent can't distinguish back/forward from forward navigation

packages/journeys/src/runtime.ts:1862 (and existing dispatchGoBack at 1827)

Both fire fireOnTransition(..., null) with default kind: "step". Telemetry consumers can only infer back/forward indirectly (exit === null + comparing to against history). Worth considering extending TransitionEvent["kind"] from "step" | "invoke" | "resume" to also include "back" | "forward" so analytics rules can filter cleanly. Out of scope for this PR — but if you ever land it, this is the natural place. (Filing the existing gap on goBack here too since the asymmetry would otherwise compound.)

3. hasRollbackSnapshot O(n) rescan on goForward

packages/journeys/src/runtime.ts:1851

record.rollbackSnapshots.push(next.snapshot);
record.hasRollbackSnapshot = record.rollbackSnapshots.some((s) => s !== undefined);

Pushing a single entry can only ever flip the flag to true, never to false. The scan is unnecessary on the goForward path:

record.rollbackSnapshots.push(next.snapshot);
if (next.snapshot !== undefined) record.hasRollbackSnapshot = true;

Minor — only matters under deep rollback stacks — but trivially correct.

4. activeChildId guard in dispatchGoForward is unreachable

packages/journeys/src/runtime.ts:1837 + test at runtime-go-forward.test.ts:280-307

The test author already notes this: goBack is itself a no-op while a child is in flight, so the future stack cannot be non-empty while activeChildId is set (and exit-driven invokes don't populate future). The guard is defensive — good — but the corresponding test really just exercises the future.length === 0 guard. If you want real coverage of the activeChildId branch you'd have to reach into the record directly. Fine to leave; just calling out that the test claim ("is a no-op while a child journey is in flight") is slightly stronger than what's actually being checked.

5. Stray empty // comment line

packages/journeys/src/runtime.ts:1287

Looks like a diff artifact — the trailing // on its own line above the if ("next" in result || …) block adds nothing. Easy cleanup.

6. Missing low-cost test cases

• goForward on an instance in status: "loading" — the status !== "active" guard would short-circuit, but it's untested directly (terminal covers the same branch).
• No explicit assertion that the persistence adapter sees the post-redo blob. The "restores history + rollbackSnapshots" test implicitly verifies record state, but a mock-adapter assertion (the same pattern your other persistence tests use) would lock the contract.

Neither is required, just nice-to-have given the otherwise-exhaustive suite.

What's good

• future correctly excluded from serialize(), reset on hydrate and on recycle in runtime.start. Transient semantics match the JSDoc.
• cachedSnapshot invalidation via notify → revision++ is correct; cachedCallbacks cleared inside both dispatch paths.
• applyTransition clears future on next | complete | abort only — invoke correctly leaves it intact, matching the browser model.
• Decision to not re-run the transition handler on redo is the right call (side-effect doubling would be a footgun) and documented prominently.
• Decision to not re-run buildInput on redo (trusting captured step.input) is internally consistent because step.input was originally built against the same state being restored. The asymmetry with dispatchGoBack (which does re-run buildInput) is explained in the inline comment — worth keeping that comment.
• Future stack is bounded by history depth at goBack time, so maxHistory is never violated by goForward pushing back into history.

LGTM — happy to see this ship once the simulator asymmetry doc nit (#1) and the stray comment (#5) are addressed.

---

Generated by Claude Code

[diogomiguel]

Thanks for the thorough review — addressed in 9abc542:

1. Simulator asymmetry doc nit — added a sentence to JourneySimulator.goForward() JSDoc making the no-op-vs-throw asymmetry with goBack explicit, so test authors don't rely on an exception to catch misordered setup.
2. hasRollbackSnapshot O(n) rescan — replaced with if (next.snapshot !== undefined) record.hasRollbackSnapshot = true. Push can only flip the flag to true here; the scan was unnecessary.
3. activeChildId test claim — renamed to "does not mutate the runtime while a child journey is in flight" and added a comment acknowledging the assertion effectively exercises the future-empty guard, not the activeChildId branch directly. Real coverage of the latter would need to mutate record.future from outside, which isn't worth the invasiveness.
4. Stray empty // line — removed (runtime.ts:1287).
   6a. goForward no-op on loading instance — added a test using a hanging-promise persistence adapter (matches the pattern at runtime.test.ts:540-565). Asserts the call doesn't throw and the instance stays in status: "loading".
   6b. Persistence-adapter post-redo assertion — added a test that pins the contract: after goBack + goForward, persistence.save is called with a blob whose step.moduleId reflects the redone position.

Not addressing #2 (extending TransitionEvent.kind to distinguish back/forward) — agree it's out of scope for this PR, noted as a follow-up.

Validated: 415/415 tests passing, typecheck + build clean.