diff --git a/Cargo.lock b/Cargo.lock index c6a930d..1d6766c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1920,7 +1920,7 @@ dependencies = [ [[package]] name = "rewind-assert" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -1935,7 +1935,7 @@ dependencies = [ [[package]] name = "rewind-cli" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -1962,7 +1962,7 @@ dependencies = [ [[package]] name = "rewind-eval" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -1977,7 +1977,7 @@ dependencies = [ [[package]] name = "rewind-mcp" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "rewind-assert", @@ -1995,7 +1995,7 @@ dependencies = [ [[package]] name = "rewind-otel" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -2020,7 +2020,7 @@ dependencies = [ [[package]] name = "rewind-proxy" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -2042,7 +2042,7 @@ dependencies = [ [[package]] name = "rewind-replay" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -2059,7 +2059,7 @@ dependencies = [ [[package]] name = "rewind-store" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -2078,7 +2078,7 @@ dependencies = [ [[package]] name = "rewind-tui" -version = "0.14.4" +version = "0.14.5" dependencies = [ "anyhow", "chrono", @@ -2096,7 +2096,7 @@ dependencies = [ [[package]] name = "rewind-web" -version = "0.14.4" +version = "0.14.5" dependencies = [ "aes-gcm", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 9047410..f7d55b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ members = [ resolver = "2" [workspace.package] -version = "0.14.4" +version = "0.14.5" edition = "2024" authors = ["Rewind Contributors"] license = "MIT" diff --git a/crates/rewind-web/src/api.rs b/crates/rewind-web/src/api.rs index 0259356..bd7ad19 100644 --- a/crates/rewind-web/src/api.rs +++ b/crates/rewind-web/src/api.rs @@ -1729,8 +1729,16 @@ async fn fork_and_edit_step( ))); } - let original_step = store.get_step_by_number(&body.source_timeline_id, body.at_step) - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("DB error: {e}")))? + // Use the engine's union view so callers can edit a step that lives + // on the parent timeline (i.e. inherited via a previous fork) — the + // bare `store.get_step_by_number` only matches owned steps and would + // 400 with "no step #N on timeline 'X'" even though the dashboard + // happily shows that step in the picker. + let engine = ReplayEngine::new(&store); + let timeline_steps = engine.get_full_timeline_steps(&body.source_timeline_id, &session.id) + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, format!("DB error: {e}")))?; + let original_step = timeline_steps.into_iter() + .find(|s| s.step_number == body.at_step) .ok_or_else(|| (StatusCode::BAD_REQUEST, format!( "no step #{} on timeline '{}'", body.at_step, body.source_timeline_id )))?; @@ -1746,7 +1754,6 @@ async fn fork_and_edit_step( })?; tl } else { - let engine = ReplayEngine::new(&store); engine.fork(&session.id, &body.source_timeline_id, body.at_step - 1, label) .map_err(|e| (StatusCode::BAD_REQUEST, format!("{e}")))? }; diff --git a/crates/rewind-web/tests/recording_api_tests.rs b/crates/rewind-web/tests/recording_api_tests.rs index cfce2c5..de97a7a 100644 --- a/crates/rewind-web/tests/recording_api_tests.rs +++ b/crates/rewind-web/tests/recording_api_tests.rs @@ -1634,3 +1634,99 @@ async fn test_fork_and_edit_step_at_zero() { assert_eq!(status, StatusCode::BAD_REQUEST, "at_step=0 should return 400"); } + +#[tokio::test] +async fn test_fork_and_edit_step_inherited_step() { + // Regression: fork-and-edit on a step that lives on the parent + // timeline (inherited via a previous fork) used to 400 because + // `store.get_step_by_number(timeline_id, n)` only matches steps + // OWNED by that timeline. The dashboard, however, happily shows + // inherited steps in the picker, so the user-facing workflow + // "open fork → edit any visible step" was silently broken for + // every step the fork didn't own. The fix routes the lookup + // through `engine.get_full_timeline_steps` (the union view). + // + // Repro recipe used in dev1 smoke (2026-04-29): + // 1. seed main with 5 steps + // 2. fork at step 2 → fork owns nothing, inherits steps 1-2 + // 3. fork-and-edit on FORK at_step=2 → 400 before fix, 201 after + let (app, _store, _tmp) = setup(); + let (sid, _root_tid) = seed_session(&app, 5).await; + + let (_, fork) = post_json(&app, &format!("/api/sessions/{sid}/fork"), json!({ + "at_step": 2, "label": "first-fork" + })).await; + let fork_tid = fork["fork_timeline_id"].as_str().unwrap(); + + // Now ask fork-and-edit to edit step #2 of THIS fork. Step #2 is + // inherited from main; the fork itself owns no steps yet. + let (status, body) = post_json(&app, &format!("/api/sessions/{sid}/fork-and-edit-step"), json!({ + "source_timeline_id": fork_tid, + "at_step": 2, + "response_body": {"edit": "of-an-inherited-step"}, + "label": "second-fork-via-inherited-edit" + })).await; + + assert_eq!( + status, StatusCode::CREATED, + "fork-and-edit on an inherited step should succeed (got: {body})" + ); + let new_fork_tid = body["fork_timeline_id"].as_str().unwrap(); + let new_step_id = body["step_id"].as_str().unwrap(); + assert!(!new_fork_tid.is_empty()); + assert!(!new_step_id.is_empty()); + + // The newly-created step lives on the new fork at step_number = at_step. + // Note: nested-fork inheritance via get_full_timeline_steps is still + // a known follow-up (only walks one parent level up), so this test + // asserts the direct invariants of the edit itself rather than the + // resulting fork's full step view. + let s = _store.lock().unwrap(); + let new_step = s.get_step(new_step_id).unwrap().unwrap(); + assert_eq!(new_step.step_number, 2); + assert_eq!(new_step.timeline_id, new_fork_tid); + + // Roundtrip the response_body through the blob store to prove the + // edited bytes actually persisted (a future refactor that reused + // the original step's response_blob would otherwise pass the + // step_number / timeline_id checks above). + let resp_bytes = s.blobs.get(&new_step.response_blob).unwrap(); + let body_back: serde_json::Value = serde_json::from_slice(&resp_bytes).unwrap(); + assert_eq!(body_back, json!({"edit": "of-an-inherited-step"})); +} + +#[tokio::test] +async fn test_fork_and_edit_step_inherited_step_below_fork_boundary() { + // Reviewer S2: the original dev1 repro was editing step 3 of a + // fork created at step 5 — i.e. a step *well below* the fork + // boundary. The first inherited-step test exercised the boundary + // step (step 2 of a fork at step 2); this variant proves the + // union-view lookup also resolves steps from deeper in the + // inherited chain (step 2 of a fork at step 4). + let (app, _store, _tmp) = setup(); + let (sid, _root_tid) = seed_session(&app, 5).await; + + let (_, fork) = post_json(&app, &format!("/api/sessions/{sid}/fork"), json!({ + "at_step": 4, "label": "deep-inherit-fork" + })).await; + let fork_tid = fork["fork_timeline_id"].as_str().unwrap(); + + let (status, body) = post_json(&app, &format!("/api/sessions/{sid}/fork-and-edit-step"), json!({ + "source_timeline_id": fork_tid, + "at_step": 2, + "response_body": {"edit": "deep-inherited"}, + "label": "edit-two-below-boundary" + })).await; + + assert_eq!( + status, StatusCode::CREATED, + "fork-and-edit on a step well below the fork boundary should succeed (got: {body})" + ); + let new_step_id = body["step_id"].as_str().unwrap(); + let s = _store.lock().unwrap(); + let new_step = s.get_step(new_step_id).unwrap().unwrap(); + assert_eq!(new_step.step_number, 2); + let resp_bytes = s.blobs.get(&new_step.response_blob).unwrap(); + let body_back: serde_json::Value = serde_json::from_slice(&resp_bytes).unwrap(); + assert_eq!(body_back, json!({"edit": "deep-inherited"})); +} diff --git a/python-mcp/pyproject.toml b/python-mcp/pyproject.toml index 3e115a2..6d2c2b2 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.3" +version = "0.13.4" 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 c810786..a5d443e 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.4" +CLI_VERSION = "0.14.5" def _get_platform_key() -> str: diff --git a/python/rewind_cli.py b/python/rewind_cli.py index 1043e7e..122ede3 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.4" +CLI_VERSION = "0.14.5" def _get_platform_key() -> str: