diff --git a/Cargo.lock b/Cargo.lock index bb7277b..6479984 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1920,7 +1920,7 @@ dependencies = [ [[package]] name = "rewind-assert" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -1935,7 +1935,7 @@ dependencies = [ [[package]] name = "rewind-cli" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -1962,7 +1962,7 @@ dependencies = [ [[package]] name = "rewind-eval" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -1977,7 +1977,7 @@ dependencies = [ [[package]] name = "rewind-mcp" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "rewind-assert", @@ -1995,7 +1995,7 @@ dependencies = [ [[package]] name = "rewind-otel" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -2020,7 +2020,7 @@ dependencies = [ [[package]] name = "rewind-proxy" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -2042,7 +2042,7 @@ dependencies = [ [[package]] name = "rewind-replay" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -2059,7 +2059,7 @@ dependencies = [ [[package]] name = "rewind-store" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -2078,7 +2078,7 @@ dependencies = [ [[package]] name = "rewind-tui" -version = "0.14.7" +version = "0.14.8" dependencies = [ "anyhow", "chrono", @@ -2096,7 +2096,7 @@ dependencies = [ [[package]] name = "rewind-web" -version = "0.14.7" +version = "0.14.8" dependencies = [ "aes-gcm", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 3010f7e..a53b84d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ members = [ resolver = "2" [workspace.package] -version = "0.14.7" +version = "0.14.8" edition = "2024" authors = ["Rewind Contributors"] license = "MIT" diff --git a/crates/rewind-web/src/dispatcher.rs b/crates/rewind-web/src/dispatcher.rs index 8f9598e..5a2c407 100644 --- a/crates/rewind-web/src/dispatcher.rs +++ b/crates/rewind-web/src/dispatcher.rs @@ -21,6 +21,8 @@ //! "job_id": "", //! "session_id": "", //! "replay_context_id": "", +//! "replay_context_timeline_id": "", +//! "at_step": , //! "base_url": "" //! } //! ``` @@ -74,12 +76,25 @@ type HmacSha256 = Hmac; /// the replay context targets — the SDK's `attach_replay_context` /// uses it to set `_timeline_id` so any cache-miss live recordings /// land in the fork rather than the original timeline. +/// +/// **`at_step`** (added 2026-04-29): the original fork-point of the +/// replay-context's timeline (i.e. the step number the user clicked +/// "Run replay" at in the dashboard). Distinct from the replay +/// context's `from_step` which the `/api/sessions/{sid}/replay-jobs` +/// handler hardcodes to 0 (see runners.rs `create_replay_job` — +/// the agent re-runs the loop from scratch so the replay-lookup +/// ordinal cursor must start at recorded step #1). Runners use +/// `at_step` to know which conversation turn the user wanted to +/// start from — needed for multi-turn replay where edits to +/// step #N's user message in turn 2+ should drive the agent at +/// iteration N (companion ray-agent change). #[derive(Debug, serde::Serialize)] struct DispatchBody<'a> { pub job_id: &'a str, pub session_id: &'a str, pub replay_context_id: &'a str, pub replay_context_timeline_id: &'a str, + pub at_step: u32, pub base_url: &'a str, } @@ -159,11 +174,16 @@ impl Dispatcher { /// **Review #154 F2:** caller passes `replay_context_timeline_id` /// so the dispatch payload can carry it and the SDK's /// `attach_replay_context` can set `_timeline_id` correctly. + /// + /// `at_step` (added 2026-04-29): the fork-point of the timeline + /// (= the step number the user clicked Run replay at). Forwarded + /// in the dispatch body for runner-side multi-turn replay. pub async fn dispatch( &self, runner: &Runner, job: &ReplayJob, replay_context_timeline_id: &str, + at_step: u32, ) -> DispatchOutcome { let webhook_url = match runner.webhook_url.as_deref() { Some(u) => u, @@ -208,6 +228,7 @@ impl Dispatcher { session_id: &job.session_id, replay_context_id, replay_context_timeline_id, + at_step, base_url: &self.base_url, }; let body_bytes = match serde_json::to_vec(&body) { diff --git a/crates/rewind-web/src/runners.rs b/crates/rewind-web/src/runners.rs index 15c482e..758cc8a 100644 --- a/crates/rewind-web/src/runners.rs +++ b/crates/rewind-web/src/runners.rs @@ -558,7 +558,7 @@ async fn create_replay_job( // 1. Resolve session, runner, and replay-context (creating fork+ // context if shape A). All store work in a single lock scope // so we don't race with deletions between checks. - let (job, runner, fork_timeline_id) = { + let (job, runner, fork_timeline_id, at_step) = { let store = state .store .lock() @@ -568,7 +568,7 @@ async fn create_replay_job( .map_err(internal)? .ok_or_else(|| not_found("session"))?; - let (runner_id, replay_context_id, fork_timeline_id) = match req { + let (runner_id, replay_context_id, fork_timeline_id, at_step) = match req { CreateReplayJobRequest::CreateAndDispatch(a) => { // Validate runner. let runner = store @@ -631,7 +631,7 @@ async fn create_replay_job( .set_replay_context_strict_match(&ctx_id, true) .map_err(internal)?; } - (a.runner_id, ctx_id, Some(fork.id)) + (a.runner_id, ctx_id, Some(fork.id), a.at_step) } CreateReplayJobRequest::ReuseContext(b) => { let runner = store @@ -691,7 +691,21 @@ async fn create_replay_job( b.replay_context_id ))); } - (b.runner_id, b.replay_context_id, Some(ctx.timeline_id)) + // Resolve at_step from the context's fork timeline. The + // dispatch payload carries the user-clicked step number + // so the runner can drive multi-turn replay; for + // ReuseContext that step is baked into the timeline's + // fork_at_step. Root timelines (parent_timeline_id = + // None) shouldn't reach here because contexts always + // target a fork; defensive fallback to 1. + let at_step = store + .get_timelines(&session.id) + .map_err(internal)? + .iter() + .find(|t| t.id == ctx.timeline_id) + .and_then(|t| t.fork_at_step) + .unwrap_or(1); + (b.runner_id, b.replay_context_id, Some(ctx.timeline_id), at_step) } }; @@ -741,7 +755,7 @@ async fn create_replay_job( None, ) .map_err(internal)?; - (job, runner, fork_timeline_id) + (job, runner, fork_timeline_id, at_step) }; // 3. Fire the dispatcher in the background. The handler returns @@ -762,9 +776,10 @@ async fn create_replay_job( let timeline_for_payload = fork_timeline_id .clone() .unwrap_or_default(); + let at_step_for_payload = at_step; tokio::spawn(async move { let outcome = dispatcher - .dispatch(&runner_clone, &job_clone, &timeline_for_payload) + .dispatch(&runner_clone, &job_clone, &timeline_for_payload, at_step_for_payload) .await; let after = { let store = match store_arc.lock() { diff --git a/crates/rewind-web/tests/replay_jobs_tests.rs b/crates/rewind-web/tests/replay_jobs_tests.rs index f3714ed..99ff73e 100644 --- a/crates/rewind-web/tests/replay_jobs_tests.rs +++ b/crates/rewind-web/tests/replay_jobs_tests.rs @@ -130,6 +130,34 @@ async fn spawn_runner_stub_accepting() -> (String, tokio::sync::mpsc::Receiver ( + String, + tokio::sync::mpsc::Receiver<(axum::http::HeaderMap, axum::body::Bytes)>, +) { + let (tx, rx) = tokio::sync::mpsc::channel(8); + let app = axum::Router::new().route( + "/wh", + axum::routing::post( + move |headers: axum::http::HeaderMap, body: axum::body::Bytes| { + let tx = tx.clone(); + async move { + let _ = tx.send((headers, body)).await; + StatusCode::ACCEPTED + } + }, + ), + ); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + (format!("http://{addr}/wh"), rx) +} + async fn spawn_runner_stub_returning_500() -> String { let app = axum::Router::new().route( "/wh", @@ -565,6 +593,94 @@ async fn shape_a_strict_match_omitted_defaults_to_warn_only() { assert!(!ctx.strict_match); } +// ── 2026-04-29: dispatch payload carries `at_step` ── + +#[tokio::test] +async fn dispatch_payload_carries_at_step() { + // Regression for the dev1 bug where the runner can't drive + // multi-turn replay because the dispatch payload doesn't tell it + // which conversation turn the user clicked on. The dispatcher now + // forwards the timeline's `fork_at_step` (= the `at_step` the + // user requested) so a future runner-side change can use it to + // reconstruct conversation history and replay from the right + // iteration. This test pins the wire-format guarantee end-to-end: + // POST shape A with at_step=4 → captured runner body has + // `"at_step": 4`. + let (api, _callbacks, store, _tmp) = setup(); + let (webhook_url, mut rx) = spawn_runner_stub_capturing_body().await; + let (runner_id, _raw) = register_runner(&store, &webhook_url); + // Need >= 4 steps so a fork at_step=4 is valid. + let (session_id, root_timeline_id) = seed_session_with_n_steps(&store, 4); + + let (status, _body) = json_post( + api, + &format!("/api/sessions/{session_id}/replay-jobs"), + json!({ + "runner_id": runner_id, + "source_timeline_id": root_timeline_id, + "at_step": 4, + }), + ) + .await; + assert_eq!(status, StatusCode::ACCEPTED); + + let (_headers, body_bytes) = + tokio::time::timeout(std::time::Duration::from_secs(3), rx.recv()) + .await + .expect("dispatcher must call the runner within 3s") + .expect("stub channel closed"); + let body_json: Value = serde_json::from_slice(&body_bytes) + .expect("dispatch body must be valid JSON"); + assert_eq!( + body_json["at_step"].as_u64(), + Some(4), + "dispatch payload must carry the user-clicked at_step (got: {body_json})" + ); + // Sanity: the other fields the SDK already reads must still be present. + assert!(body_json["job_id"].is_string()); + assert!(body_json["session_id"].is_string()); + assert!(body_json["replay_context_id"].is_string()); + assert!(body_json["replay_context_timeline_id"].is_string()); + assert!(body_json["base_url"].is_string()); +} + +#[tokio::test] +async fn dispatch_payload_at_step_for_reuse_context_reads_fork_at_step() { + // When dispatching against a pre-existing replay context (shape B), + // there's no `at_step` on the request body — but the dispatch + // payload still needs `at_step` so the runner can drive multi-turn + // replay. The handler reads it from the context's fork timeline's + // `fork_at_step` field. For root timelines (no parent) the + // fallback is 1. + let (api, _callbacks, store, _tmp) = setup(); + let (webhook_url, mut rx) = spawn_runner_stub_capturing_body().await; + let (runner_id, _raw) = register_runner(&store, &webhook_url); + // seed_session_and_context creates a context against the ROOT + // timeline (parent_timeline_id=None, fork_at_step=None). The + // handler should default at_step to 1. + let (session_id, _root, ctx_id) = seed_session_and_context(&store); + + let (status, _body) = json_post( + api, + &format!("/api/sessions/{session_id}/replay-jobs"), + json!({"runner_id": runner_id, "replay_context_id": ctx_id}), + ) + .await; + assert_eq!(status, StatusCode::ACCEPTED); + + let (_headers, body_bytes) = + tokio::time::timeout(std::time::Duration::from_secs(3), rx.recv()) + .await + .expect("dispatcher must call the runner within 3s") + .expect("stub channel closed"); + let body_json: Value = serde_json::from_slice(&body_bytes).unwrap(); + assert_eq!( + body_json["at_step"].as_u64(), + Some(1), + "ReuseContext path: at_step falls back to 1 when timeline has no fork_at_step (got: {body_json})" + ); +} + // ── Review #154 F3: Shape B cursor + TTL validation ── #[tokio::test] @@ -622,6 +738,13 @@ async fn shape_b_rejects_expired_context() { // Helper: session with one recorded step so Shape A's fork(at_step=1) // has something to fork from. fn seed_session_with_step(store: &Arc>) -> (String, String) { + seed_session_with_n_steps(store, 1) +} + +/// Seed a session + root timeline + N owned LLM-call steps. Returns +/// `(session_id, timeline_id)`. Used by tests that need a session +/// with enough steps to fork at a meaningful at_step value. +fn seed_session_with_n_steps(store: &Arc>, n: u32) -> (String, String) { use rewind_store::{Session, SessionSource, SessionStatus, Step, StepStatus, Timeline}; let s = store.lock().unwrap(); let session = Session { @@ -641,11 +764,13 @@ fn seed_session_with_step(store: &Arc>) -> (String, String) { let timeline = Timeline::new_root(&session.id); s.create_session(&session).unwrap(); s.create_timeline(&timeline).unwrap(); - let mut step = Step::new_llm_call(&timeline.id, &session.id, 1, "stub-model"); - step.status = StepStatus::Success; - step.duration_ms = 10; - s.create_step(&step).unwrap(); - s.update_session_stats(&session.id, 1, 0).unwrap(); + for i in 1..=n { + let mut step = Step::new_llm_call(&timeline.id, &session.id, i, "stub-model"); + step.status = StepStatus::Success; + step.duration_ms = 10; + s.create_step(&step).unwrap(); + } + s.update_session_stats(&session.id, n, 0).unwrap(); (session.id, timeline.id) } diff --git a/docs/runners.md b/docs/runners.md index 1313e41..3d0a198 100644 --- a/docs/runners.md +++ b/docs/runners.md @@ -193,6 +193,42 @@ The body accepts two shapes: Returns `202 Accepted` with `{job_id, replay_context_id, fork_timeline_id?, state, dispatch_deadline_at}`. +### Dispatch wire format (server → runner) + +The dispatcher POSTs to the runner's webhook URL: + +```http +POST +Content-Type: application/json +X-Rewind-Job-Id: +X-Rewind-Signature: sha256= +X-Rewind-Timestamp: +User-Agent: rewind-dispatcher/ + +{ + "job_id": "", + "session_id": "", + "replay_context_id": "", + "replay_context_timeline_id": "", + "at_step": , + "base_url": "" +} +``` + +**`at_step`** (added v0.14.8) is the original fork-point of the replay-context's timeline — i.e. the step number the user clicked Run replay at. Distinct from the replay context's `from_step` (always `0`; the agent re-runs from scratch). Runners use it for **multi-turn replay**: when `at_step > 1`, the runner should fetch the source timeline's steps `1..at_step-1` from rewind, reconstruct conversation history, and invoke the agent at the right turn so edits to user messages in turn 2+ actually take effect. The current ray-agent runner doesn't yet consume this field — companion ray-agent PR adds the runner-side reconstruction. + +Older runner SDK versions (pre v0.14.8) tolerate the new field gracefully — Python's `from_envelope` ignores unknown keys, and the HMAC verification works against the raw `request.body` so the new field is part of the signed input on both ends. + +### Timeline-context contract (dashboard) + +All four step-action buttons in the dashboard — **Edit**, **Fork from here**, **Set up replay**, and **Run replay** — source their action from the user's *currently-selected* timeline, not the step's physical owner. The fallback chain is: + +``` +contextTimelineId = selectedTimelineId ?? rootTimelineId ?? step.timeline_id +``` + +This matters for inherited steps shown on a fork: clicking any of these buttons routes the action through the fork lineage, not the parent (often `main`). Combined with the dedup in `engine.get_full_timeline_steps` (v0.14.7+), the picker shows owned-over-inherited rows so the user clicks the right step in the first place, and the action lands on the right timeline. + --- ## State machine diff --git a/python-mcp/pyproject.toml b/python-mcp/pyproject.toml index 1f897e2..8c904b1 100644 --- a/python-mcp/pyproject.toml +++ b/python-mcp/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "rewind-mcp" -version = "0.13.6" +version = "0.13.7" description = "MCP server for Rewind — the time-travel debugger for AI agents." readme = "README.md" license = "MIT" diff --git a/python-mcp/rewind_mcp_cli.py b/python-mcp/rewind_mcp_cli.py index 4c8e3eb..57877b4 100644 --- a/python-mcp/rewind_mcp_cli.py +++ b/python-mcp/rewind_mcp_cli.py @@ -29,7 +29,7 @@ # Must match the Rust workspace version in Cargo.toml. # Update this when a new GitHub Release is published with new binaries. -CLI_VERSION = "0.14.7" +CLI_VERSION = "0.14.8" def _get_platform_key() -> str: diff --git a/python/pyproject.toml b/python/pyproject.toml index 8fa9aee..27385c6 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "rewind-agent" -version = "0.15.1" +version = "0.15.2" description = "Chrome DevTools for AI agents — record, inspect, fork, replay, diff." readme = "README.md" license = "MIT" diff --git a/python/rewind_agent/__init__.py b/python/rewind_agent/__init__.py index fb14082..f6bb1ea 100644 --- a/python/rewind_agent/__init__.py +++ b/python/rewind_agent/__init__.py @@ -113,4 +113,4 @@ def import_from_langfuse(trace_id: str, **kwargs) -> str: return _import(trace_id, **kwargs) -__version__ = "0.15.1" +__version__ = "0.15.2" diff --git a/python/rewind_agent/runner.py b/python/rewind_agent/runner.py index 10931a5..3e63852 100644 --- a/python/rewind_agent/runner.py +++ b/python/rewind_agent/runner.py @@ -142,6 +142,19 @@ class DispatchPayload: the replay context targets — runners pass it to ``ExplicitClient.attach_replay_context`` so live cache misses record into the fork. + + **Added 2026-04-29:** ``at_step`` is the original fork-point of + the replay-context's timeline — i.e. the step number the user + clicked Run replay at in the dashboard. Distinct from the + replay-context's ``from_step`` (always 0 because the agent + re-runs from scratch). Runners use ``at_step`` to drive + multi-turn replay: when ``at_step > 1``, fetch the source + timeline's steps 1..at_step-1, reconstruct the conversation + history, and invoke the agent at the right turn so edits to + user messages in turn 2+ actually take effect. + + Defaults to ``None`` for back-compat with older servers (pre + v0.14.8) that don't send the field. """ job_id: str @@ -149,6 +162,7 @@ class DispatchPayload: replay_context_id: str replay_context_timeline_id: str base_url: str + at_step: Optional[int] = None @classmethod def from_json(cls, body: dict[str, Any]) -> "DispatchPayload": @@ -162,6 +176,10 @@ def from_json(cls, body: dict[str, Any]) -> "DispatchPayload": # being set, with the documented consequence. replay_context_timeline_id=body.get("replay_context_timeline_id", ""), base_url=body["base_url"], + # Tolerate older servers (pre v0.14.8) that don't send + # at_step. Runners that depend on it for multi-turn + # replay should branch on `payload.at_step is not None`. + at_step=body.get("at_step"), ) diff --git a/python/rewind_cli.py b/python/rewind_cli.py index 52d031b..27acb03 100644 --- a/python/rewind_cli.py +++ b/python/rewind_cli.py @@ -32,7 +32,7 @@ # The native CLI binary version — independent of the Python SDK version. # Update this when a new GitHub Release is published with new binaries. -CLI_VERSION = "0.14.7" +CLI_VERSION = "0.14.8" def _get_platform_key() -> str: diff --git a/python/tests/test_runner.py b/python/tests/test_runner.py index 4777929..74307bb 100644 --- a/python/tests/test_runner.py +++ b/python/tests/test_runner.py @@ -349,6 +349,62 @@ def test_dispatch_payload_tolerates_missing_timeline_id_for_back_compat() -> Non assert payload.replay_context_timeline_id == "" +def test_dispatch_payload_decodes_at_step() -> None: + """v0.14.8+ servers include `at_step` in the dispatch body — the + fork-point of the replay-context's timeline. Runners use it to + drive multi-turn replay (start the agent at the right turn so + edits to user messages in turn 2+ take effect).""" + body = { + "job_id": "j", + "session_id": "s", + "replay_context_id": "r", + "replay_context_timeline_id": "tl-fork", + "at_step": 4, + "base_url": "http://x.example", + } + payload = runner.DispatchPayload.from_json(body) + assert payload.at_step == 4 + + +def test_dispatch_payload_at_step_defaults_to_none_for_back_compat() -> None: + """Older servers (pre v0.14.8) don't send `at_step`. The SDK + decodes the body anyway; runners that need at_step branch on + `payload.at_step is not None` and fall back to single-turn + behavior when it's missing.""" + body = { + "job_id": "j", + "session_id": "s", + "replay_context_id": "r", + "replay_context_timeline_id": "tl-fork", + "base_url": "http://x.example", + } + payload = runner.DispatchPayload.from_json(body) + assert payload.at_step is None + + +def test_dispatch_payload_tolerates_extra_unknown_keys() -> None: + """Forward-compat: future server versions may add fields. Older + runner SDKs hitting newer servers must keep working — extra + keys in the body are ignored, not rejected. This pins the + contract that lets us add fields without breaking deployed + runners (no HMAC concern: the server signs the wire bytes; + the runner verifies against raw `request.body`, so extra + fields are part of the signed payload on both ends).""" + body = { + "job_id": "j", + "session_id": "s", + "replay_context_id": "r", + "replay_context_timeline_id": "tl-fork", + "at_step": 4, + "base_url": "http://x.example", + "future_field_added_in_v0_14_99": "ignored", + "another_future_field": {"nested": [1, 2, 3]}, + } + payload = runner.DispatchPayload.from_json(body) + assert payload.job_id == "j" + assert payload.at_step == 4 + + # ────────────────────────────────────────────────────────────────── # asgi_handler — end-to-end with a mocked event endpoint # ────────────────────────────────────────────────────────────────── diff --git a/web/src/components/StepDetailPanel.test.tsx b/web/src/components/StepDetailPanel.test.tsx index 29e2eb3..d4d54e0 100644 --- a/web/src/components/StepDetailPanel.test.tsx +++ b/web/src/components/StepDetailPanel.test.tsx @@ -1,8 +1,9 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { render, screen, fireEvent, cleanup } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { StepDetailPanel } from './StepDetailPanel' import { api } from '@/lib/api' +import { useStore } from '@/hooks/use-store' import type { StepDetail } from '@/types/api' // Stub the API so the test doesn't try to network. The panel gates on @@ -12,9 +13,17 @@ vi.mock('@/lib/api', () => ({ api: { stepDetail: vi.fn(), health: vi.fn().mockResolvedValue({ status: 'ok', version: '0.14.4', allow_main_edits: false }), + // Session mock includes BOTH a root timeline AND a fork. Existing + // tests that don't set selectedTimelineId still get fallback=root + // (=tl-main), so their data-tid assertions keep passing. The new + // timeline-context tests set selectedTimelineId='tl-fork' to drive + // the context-routing branch. session: vi.fn().mockResolvedValue({ session: { id: 'sess-1', name: 'test', created_at: '', updated_at: '', status: 'Completed', total_steps: 4, total_tokens: 0, metadata: {} }, - timelines: [{ id: 'tl-main', session_id: 'sess-1', parent_timeline_id: null, fork_at_step: null, created_at: '', label: 'main' }], + timelines: [ + { id: 'tl-main', session_id: 'sess-1', parent_timeline_id: null, fork_at_step: null, created_at: '', label: 'main' }, + { id: 'tl-fork', session_id: 'sess-1', parent_timeline_id: 'tl-main', fork_at_step: 2, created_at: '', label: 'fork' }, + ], }), cascadeCount: vi.fn().mockResolvedValue({ deleted_downstream_count: 2, on_main: true }), patchStep: vi.fn().mockResolvedValue({ step_id: 'step-abc', deleted_downstream_count: 2 }), @@ -25,16 +34,19 @@ vi.mock('@/lib/api', () => ({ // The two modal components mount as siblings; we don't care what they // render — just that the right one becomes visible when its button // fires. Stubbing them out keeps the test focused on header buttons. +// All three stubs capture `timelineId`/`sourceTimelineId` as data-tid +// so timeline-context routing tests can assert which timeline the +// dashboard passed in. vi.mock('./ForkModal', () => ({ - ForkModal: ({ isOpen, sessionId, atStep }: { isOpen: boolean; sessionId: string; atStep: number }) => + ForkModal: ({ isOpen, sessionId, timelineId, atStep }: { isOpen: boolean; sessionId: string; timelineId: string; atStep: number }) => isOpen ? ( -
+
) : null, })) vi.mock('./ReplaySetupModal', () => ({ - ReplaySetupModal: ({ isOpen, sessionId, atStep }: { isOpen: boolean; sessionId: string; atStep: number }) => + ReplaySetupModal: ({ isOpen, sessionId, timelineId, atStep }: { isOpen: boolean; sessionId: string; timelineId: string; atStep: number }) => isOpen ? ( -
+
) : null, })) vi.mock('./RunReplayButton', () => ({ @@ -196,3 +208,61 @@ describe('StepDetailPanel — Step editing', () => { expect(screen.getByTitle(/edit response/i)).toBeTruthy() }) }) + +// ────────────────────────────────────────────────────────────────── +// Timeline-context routing for Fork / Set up replay / Run replay +// +// Bug class fixed in this PR (and previously in #161 for the Edit +// button): all four step-action buttons must source their action +// from the user's SELECTED timeline, not the step's physical owner. +// Otherwise inherited steps shown on a fork dispatch against the +// parent (often main), discarding the user's fork lineage. +// +// `contextTimelineId` fallback chain in StepDetailPanel: +// selectedTimelineId ?? rootTimelineId ?? step.timeline_id +// +// Each modal stub reflects the prop it was given as `data-tid`. +// ────────────────────────────────────────────────────────────────── + +describe('StepDetailPanel — timeline-context routing for action buttons', () => { + beforeEach(() => { + cleanup() + vi.mocked(api.stepDetail).mockResolvedValue(makeStep()) + // Drive the SELECTED-timeline branch: user is viewing tl-fork + // even though the step physically lives on tl-main (i.e. it's + // an inherited step). + useStore.setState({ selectedTimelineId: 'tl-fork' }) + }) + + afterEach(() => { + // Reset so other test suites that rely on null get a clean slate. + useStore.setState({ selectedTimelineId: null }) + }) + + it('ForkModal receives the SELECTED timeline (not step.timeline_id) when they differ', async () => { + renderWithClient() + fireEvent.click(await screen.findByRole('button', { name: /fork from here/i })) + const modal = await screen.findByRole('dialog', { name: 'ForkModal-stub' }) + expect(modal.getAttribute('data-tid')).toBe('tl-fork') + expect(modal.getAttribute('data-tid')).not.toBe('tl-main') + }) + + it('ReplaySetupModal receives the SELECTED timeline (not step.timeline_id) when they differ', async () => { + renderWithClient() + fireEvent.click(await screen.findByRole('button', { name: /set up replay/i })) + const modal = await screen.findByRole('dialog', { name: 'ReplaySetupModal-stub' }) + expect(modal.getAttribute('data-tid')).toBe('tl-fork') + expect(modal.getAttribute('data-tid')).not.toBe('tl-main') + }) + + it('ReplayJobModal receives the SELECTED timeline (not step.timeline_id) when they differ', async () => { + renderWithClient() + fireEvent.click(await screen.findByRole('button', { name: /run replay/i })) + const modal = await screen.findByRole('dialog', { name: 'ReplayJobModal-stub' }) + // Pre-fix this would have been 'tl-main' (step.timeline_id), + // dispatching the replay against main instead of the user's + // fork — the original dev1 repro on session ray-agent-85551571. + expect(modal.getAttribute('data-tid')).toBe('tl-fork') + expect(modal.getAttribute('data-tid')).not.toBe('tl-main') + }) +}) diff --git a/web/src/components/StepDetailPanel.tsx b/web/src/components/StepDetailPanel.tsx index 821b3ae..88e9a4b 100644 --- a/web/src/components/StepDetailPanel.tsx +++ b/web/src/components/StepDetailPanel.tsx @@ -253,7 +253,7 @@ export function StepDetailPanel({ stepId }: { stepId: string }) { isOpen onClose={() => setModalMode(null)} sessionId={step.session_id} - timelineId={step.timeline_id} + timelineId={contextTimelineId} atStep={step.step_number} /> )} @@ -262,14 +262,14 @@ export function StepDetailPanel({ stepId }: { stepId: string }) { isOpen onClose={() => setModalMode(null)} sessionId={step.session_id} - timelineId={step.timeline_id} + timelineId={contextTimelineId} atStep={step.step_number} /> )} {modalMode === 'runReplay' && ( setModalMode(null)} />