fix(replay): route step-action buttons through selected timeline + dispatch carries at_step

[risjai]

Summary

Two related bugs surfaced in dev1 testing on session ray-agent-85551571:

Bug A — Three step-action buttons (Fork, Set up replay, Run replay) sourced their action from step.timeline_id (the step's physical owner) instead of the user's selected timeline. Same class of bug PR #161 fixed for the Edit button.

Live repro: forked at step 1, edited the user message, clicked Run replay → resulting fork was parented to main, not to the user's edit fork. The agent re-ran the original "aceml cluster" question instead of the edited "hepo cluster" question.

Bug B (server + SDK side) — The dispatch payload doesn't carry at_step, so runners can't drive multi-turn replay. Today the runner re-asks the session's first user message; for sessions with multiple conversation turns, edits to user messages in turn 2+ are silently ignored.

Scope

• Bug A: complete (frontend fix)
• Bug B: server + SDK wire-format only. The companion ray-agent PR consumes the new field.

Server changes

• crates/rewind-web/src/dispatcher.rs — DispatchBody adds at_step: u32 + docstring + wire-format example
• crates/rewind-web/src/runners.rs — create_replay_job resolves at_step (NewContext: from request; ReuseContext: from timeline's fork_at_step), passes through to dispatcher.dispatch

SDK changes

• python/rewind_agent/runner.py — DispatchPayload adds `at_step: Optional[int] = None` with docstring; `from_json` reads it with `.get()` for back-compat (older servers default to None)

Frontend changes

• web/src/components/StepDetailPanel.tsx — three `step.timeline_id` references replaced with `contextTimelineId` (the existing fallback chain selectedTimelineId ?? rootTimelineId ?? step.timeline_id already computed at component scope for the Edit hook)

Tests

• replay_jobs_tests: +2 (dispatch_payload_carries_at_step for shape A, dispatch_payload_at_step_for_reuse_context_reads_fork_at_step for shape B). Mock runner stub captures the body and asserts at_step matches.
• test_runner.py: +3 (decodes_at_step, defaults_to_none_for_back_compat, tolerates_extra_unknown_keys — pins forward-compat for older runners hitting newer servers)
• StepDetailPanel.test.tsx: +3 (Fork/ReplaySetup/ReplayJob receive selectedTimelineId when set). Existing fallback test (data-tid = step.timeline_id when selectedTimelineId is null) kept as the fallback-path coverage.

Suite	Before	After	Delta
cargo test rewind-web --test replay_jobs_tests	20	22	+2
cargo test rewind-web --test recording_api_tests	68	68	0
cargo test rewind-replay --lib	13	13	0
npm test (web)	183	186	+3
pytest tests/test_runner.py	29	32	+3

Wire-format change implication

Adding at_step to DispatchBody is a wire-format addition. No HMAC concern: the server signs the wire bytes; the runner verifies HMAC(secret, raw_body) == header. Both sides use the same bytes, so the new field is signed and verified consistently. Older runner SDKs tolerate the new field (Python from_envelope ignores unknown keys — pinned by the new test).

Docs

• docs/runners.md gains a "Dispatch wire format" section documenting at_step semantics + the timeline-context contract for all four step-action buttons.

Out of scope (companion ray-agent PR)

After this PR ships, a separate small PR in agentoptics/ray-agent:

• use-cases/ray-agent/src/serve/rewind_replay.py handler: when payload.at_step > 1, fetch source timeline steps 1..at_step-1 from rewind, reconstruct conversation_history from each step's request_body.messages, extract the user message at at_step as the new question, invoke react_agent.run(question=..., conversation_history=..., ...).

Version bump

Track 1 (rewind workspace + CLI binary): 0.14.7 → 0.14.8 across Cargo.toml, Cargo.lock, python/rewind_cli.py, python-mcp/pyproject.toml, python-mcp/rewind_mcp_cli.py.
Track 2 (rewind-agent SDK on PyPI): 0.15.1 → 0.15.2 (python/pyproject.toml, python/rewind_agent/__init__.py).
Track 3 (rewind-mcp): 0.13.6 → 0.13.7 (per-PR convention).

Test plan

• cargo build -p rewind-web clean
• cargo test -p rewind-web --test recording_api_tests 68/68
• cargo test -p rewind-web --test replay_jobs_tests 22/22
• cargo test -p rewind-replay --lib 13/13
• npm test 186/186
• pytest tests/test_runner.py 32/32
• No linter errors
• Live verification in dev1 post-release: dispatch payload contains at_step, dashboard's Run replay button on a fork dispatches against the fork (not main)

Made with Cursor

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
rewind	Ready	Preview, Comment	Apr 29, 2026 11:48am

[risjai]

Review: fix(replay): route step-action buttons through selected timeline + dispatch carries at_step

Clean PR that fixes both bugs from the plan. Bug A is a 3-line frontend fix (same pattern as #161). Bug B's server+SDK side is well-structured with proper back-compat handling. Test coverage is thorough: 2 server, 3 SDK, 3 frontend.

Findings

#	Severity	Finding
1	Low	Docstring on DispatchBody.at_step says from_step is "always 0" — misleading
2	Low	Shape B resolves at_step from timeline.fork_at_step, not from replay_context.from_step
3	Info	HMAC paragraph correctly removed from risk section
4	Info	Frontend tests use useStore.setState with afterEach cleanup — good pattern

L1 — Docstring says from_step is "always 0"

dispatcher.rs line 83-86:

Distinct from the replay context's `from_step` which is always 0
(the agent re-runs from scratch).

from_step is NOT always 0. It's set to the user's at_step when the replay context is created (see create_replay_context which takes body.from_step). The confusion is between the replay cursor (starts at 0 and advances) vs the replay fork point (the step the user clicked). The docstring should read: "Distinct from the replay context's from_step which tracks where the cursor starts in the replay-lookup ordinal sequence."

L2 — at_step source inconsistency between shape A and B

Shape A uses a.at_step directly (from the request body — the step the user clicked). Shape B resolves it from timeline.fork_at_step:

let at_step = store.get_timelines(&session.id)...
    .find(|t| t.id == ctx.timeline_id)
    .and_then(|t| t.fork_at_step)
    .unwrap_or(1);

In practice, fork_at_step == the original at_step used to create the fork, so they're equal. But the plan review suggested resolving from the replay context's from_step (the caller already has the context in ctx). Using ctx.from_step would be more direct and avoid the extra get_timelines scan:

let at_step = ctx.from_step;

Not blocking — the current code is correct. Just a slightly longer path to the same value.

I3 — Wire-format section

The PR body correctly documents: "No HMAC concern: the server signs the wire bytes; the runner verifies HMAC(secret, raw_body) == header." This is the corrected understanding from the plan review. Good.

I4 — Frontend test pattern

The useStore.setState({ selectedTimelineId: 'tl-fork' }) + afterEach cleanup pattern is clean and avoids leaking state between test suites. The existing fallback test (no selectedTimelineId set → falls back to root → data-tid = tl-main) is preserved. Good.

What I verified

• The three step.timeline_id → contextTimelineId substitutions at lines 256, 265, 272 are exactly the right changes
• contextTimelineId is at component scope (line 39), so all three modal props can reference it
• DispatchBody.at_step is u32 (non-optional on the server — always known), while the SDK's DispatchPayload.at_step is Optional[int] (back-compat for older servers). Correct asymmetry.
• The spawn_runner_stub_capturing_body test helper captures raw bytes so assertions can verify the exact wire payload
• Both shape A and shape B dispatch tests assert at_step in the payload
• Forward-compat test (tolerates_extra_unknown_keys) pins the contract that lets future field additions not break deployed runners
• Version bumps: Track 1 (0.14.7→0.14.8), Track 2 (0.15.1→0.15.2), MCP (0.13.6→0.13.7) — all correct

Verdict

Ship it. The two LOWs are docstring/style — no correctness impact. The PR cleanly closes Bug A and ships the wire-format foundation for Bug B's runner-side fix. After this merges, the ray-agent companion PR can consume payload.at_step for multi-turn replay.

[risjai]

Thanks for the review. Folded L1 in as e0b7c49 — the DispatchBody.at_step docstring now points at runners.rs create_replay_job (which is what actually hardcodes from_step=0) instead of asserting it as an immutable property of the column.

On L2 — I'd push back. ctx.from_step would not work because the API endpoint always passes 0 to create_replay_context (see runners.rs:621-623 and the comment block above it explaining why). Substituting let at_step = ctx.from_step would set at_step = 0 for every ReuseContext dispatch and silently break the multi-turn-replay use case the field exists for.

timeline.fork_at_step is the only place where the user-clicked step is preserved across the round trip (it's baked into the timeline row at fork-creation time). The extra get_timelines scan is the cost of paying that — small (single SQL query, already cached SQLite page), and the alternative would mean either:

• adding a new at_step column to replay_contexts (schema migration), or
• changing the semantic of from_step to mean "user fork-point" instead of "cursor start" (would require auditing every replay-lookup call site)

Both are bigger changes for the same observable behavior. Happy to revisit if a column migration becomes worth it for other reasons.

CI re-running on the new commit. Will merge once green.