diff --git a/crates/dashboard-app/src/captures.rs b/crates/dashboard-app/src/captures.rs index 7557d2f..62c8eb0 100644 --- a/crates/dashboard-app/src/captures.rs +++ b/crates/dashboard-app/src/captures.rs @@ -1886,3 +1886,110 @@ fn captures_work_reload_stopped() { save(&mut cx, handle, &out, "stopped-work-2-enter"); assert_eq!(editing(&mut cx), Some(255)); } + +/// Issue 94: the Projects re-reads take pmd's wrapped `GET /v1/projects/{slug}` answer. A stale-version save reaches +/// the conflict and Reapply saves on the re-read version; a create that timed out but landed opens as the stored +/// project. Each state is saved, then checked, so the old code still writes the state it gets wrong. +#[test] +#[ignore = "writes issue 94 Projects re-read states offscreen"] +fn captures_projects_reread() { + use crate::projects_tests::{FakeProjects, Folder}; + use dashboard_core::write::{CreateCheck, Precondition, WriteError}; + use serde_json::json; + let out = PathBuf::from(std::env::var_os("CAPTURE_DIR").expect("CAPTURE_DIR")); + std::fs::create_dir_all(&out).unwrap(); + let mut cx = headless(); + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, manage) = open_manage_dialog( + &mut cx, + backend.clone(), + Some(folder.config_dir().join("config.toml")), + dashboard_core::write::Feature::Projects, + ); + let click = |cx: &mut HeadlessAppContext, id: &str| { + let id = gpui_kit::SharedString::from(id.to_owned()); + cx.update_window(handle, |_, window, cx| window.click(id, cx)).unwrap(); + settle(cx); + }; + let form = |cx: &mut HeadlessAppContext| { + cx.update(|cx| { + manage.read(cx).projects().unwrap().editor_form().map(|form| { + ( + form.value("slug").to_owned(), + form.version(), + form.message().map(str::to_owned), + form.conflict_state().map(|c| { + c.differences() + .into_iter() + .map(|(field, mine, theirs)| format!("{field}: {mine} / {theirs}")) + .collect::>() + }), + ) + }) + }) + }; + + // gh-x at P3, version 2: pick P1 while someone else changes its goal. + click(&mut cx, "project-row-3"); + click(&mut cx, "project-priority-1"); + backend.fail_next(WriteError::StaleVersion, |rows| { + let row = rows.iter_mut().find(|row| row["slug"] == "gh-x").unwrap(); + row["goal"] = json!("Ship the status schema v2"); + row["version"] = json!(6); + }); + key(&mut cx, handle, &["ctrl-enter"]); + settle(&mut cx); + save(&mut cx, handle, &out, "reread-projects-1-conflict"); + let (slug, version, _, conflict) = form(&mut cx).unwrap(); + assert_eq!((slug.as_str(), version), ("gh-x", Some(2))); + assert_eq!( + conflict.expect("the conflict panel"), + [ + "goal: Maintenance: keep healthy, no new initiatives. / Ship the status schema v2", + "priority: 1 / 3", + ] + ); + + // Reapply mine: one PUT on the re-read version. + click(&mut cx, "conflict-reapply"); + settle(&mut cx); + save(&mut cx, handle, &out, "reread-projects-2-reapplied"); + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 2); + assert_eq!(writes[1].precondition, Precondition::IfMatch(6)); + assert_eq!(form(&mut cx).unwrap().1, Some(7)); + + // Add yahtzee; the create times out, but pmd stored it. + click(&mut cx, "projects-back"); + click(&mut cx, "projects-add"); + let repo = cx.update(|cx| manage.read(cx).input("repo").unwrap()); + cx.update_window(handle, |_, window, cx| { + repo.update(cx, |input, cx| input.set_value("hemsoft-dev/yahtzee", window, cx)) + }) + .unwrap(); + settle(&mut cx); + click(&mut cx, "project-priority-3"); + backend.fail_next( + WriteError::Timeout { + create: Some(CreateCheck::Exists), + }, + |rows| { + rows.push( + json!({"slug": "yahtzee", "repo": "hemsoft-dev/yahtzee", "priority": 3, "goal": "", + "notes": "", "requires": [], "tracked": true, "riskLevel": 5, "source": "api", "version": 1}), + ) + }, + ); + key(&mut cx, handle, &["ctrl-enter"]); + settle(&mut cx); + save(&mut cx, handle, &out, "reread-projects-3-created"); + assert_eq!(form(&mut cx).unwrap(), ("yahtzee".to_owned(), Some(1), None, None)); + assert_eq!(backend.writes.lock().unwrap().len(), 3, "one create, never resent"); + + // Back: the list has it. + click(&mut cx, "projects-back"); + save(&mut cx, handle, &out, "reread-projects-4-list"); + let listed = cx.update(|cx| manage.read(cx).projects().unwrap().visible_slugs(cx)); + assert!(listed.contains(&"yahtzee".to_owned()), "{listed:?}"); +} diff --git a/crates/dashboard-app/src/manage/projects.rs b/crates/dashboard-app/src/manage/projects.rs index e884af7..3a1c859 100644 --- a/crates/dashboard-app/src/manage/projects.rs +++ b/crates/dashboard-app/src/manage/projects.rs @@ -637,7 +637,7 @@ impl Manage { .background_executor() .spawn(async move { reader.read(&path) }) .await - .and_then(|value| Project::from_json(&value)); + .and_then(|value| Project::one_from_json(&value)); let _ = this.update_in(cx, |manage, window, cx| { let writable = manage.projects_writable(); let Some(projects) = manage.projects_of(id) else { diff --git a/crates/dashboard-app/src/projects_tests.rs b/crates/dashboard-app/src/projects_tests.rs index 8fd69fc..d74f7e2 100644 --- a/crates/dashboard-app/src/projects_tests.rs +++ b/crates/dashboard-app/src/projects_tests.rs @@ -49,11 +49,15 @@ pub(crate) fn sample_projects() -> Vec { type Change = Box) + Send>; /// An in-memory pmd for projects: the list with `all=1`, single rows, and `PUT` with `If-Match` or `If-None-Match`. +/// A single row comes back as pmd's `GET /v1/projects/{slug}` sends it, `{"project": {...}, "items": [...]}` (#94). pub(crate) struct FakeProjects { scopes: Vec, pub rows: Mutex>, pub writes: Mutex>, pub fail_next: Mutex)>>, + /// What the next single-row read answers instead, such as a malformed body. + pub next_read: Mutex>, + pub reads: AtomicUsize, } impl FakeProjects { @@ -63,9 +67,16 @@ impl FakeProjects { rows: Mutex::new(sample_projects()), writes: Mutex::default(), fail_next: Mutex::default(), + next_read: Mutex::default(), + reads: AtomicUsize::new(0), }) } + /// The next single-row read answers `value`, whatever pmd holds. + pub(crate) fn answer_next_read(&self, value: Value) { + *self.next_read.lock().unwrap() = Some(value); + } + pub(crate) fn fail_next(&self, error: WriteError, elsewhere: impl FnOnce(&mut Vec) + Send + 'static) { *self.fail_next.lock().unwrap() = Some((error, Some(Box::new(elsewhere)))); } @@ -95,12 +106,18 @@ impl WriteBackend for FakeProjects { Ok(()) } fn read(&self, path: &str) -> Result { + self.reads.fetch_add(1, Ordering::SeqCst); let slug = path.strip_prefix("v1/projects/").ok_or(WriteError::NotFound)?; + if let Some(answer) = self.next_read.lock().unwrap().take() { + return Ok(answer); + } let rows = self.rows.lock().unwrap(); - rows.iter() + let project = rows + .iter() .find(|row| row["slug"] == slug) .cloned() - .ok_or(WriteError::NotFound) + .ok_or(WriteError::NotFound)?; + Ok(json!({"project": project, "items": []})) } fn list(&self, path: &str, query: &[(String, String)]) -> Result { assert_eq!(path, "v1/projects"); @@ -493,10 +510,89 @@ async fn changed_elsewhere_shows_both_values_and_reapplies_on_the_new_version(cx assert_eq!(text_of(cx, handle, "conflict-priority"), "Priority: yours 1, now 2"); click(cx, handle, "conflict-reapply"); settle(cx, handle, &view).await; - assert_eq!(backend.writes.lock().unwrap()[1].precondition, Precondition::IfMatch(6)); + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 2, "the conflict's one write, then Reapply's"); + assert_eq!(writes[1].precondition, Precondition::IfMatch(6), "the re-read version"); assert_eq!(backend.row("gh-x")["priority"], json!(1)); } +/// The editor's form: its message, version and whether it has unsaved edits. +fn form_state(cx: &mut TestAppContext, manage: &Entity) -> (Option, Option, bool) { + cx.update(|cx| { + let form = manage.read(cx).projects().unwrap().editor_form().unwrap(); + (form.message().map(str::to_owned), form.version(), form.is_dirty()) + }) +} + +#[gpui_kit::test] +async fn changed_elsewhere_can_discard_mine_for_the_reread_values(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + edit(cx, handle, &manage, "gh-x"); + set_input(cx, handle, &manage, "notes", "mine"); + backend.fail_next(WriteError::StaleVersion, |rows| { + let row = rows.iter_mut().find(|row| row["slug"] == "gh-x").unwrap(); + row["goal"] = json!("Theirs"); + row["version"] = json!(6); + }); + click(cx, handle, "project-save"); + settle(cx, handle, &view).await; + frame(cx, handle); + assert_eq!(text_of(cx, handle, "conflict-notes"), "Notes: yours mine, now (empty)"); + assert_eq!( + text_of(cx, handle, "conflict-goal"), + "Goal: yours Maintenance: keep healthy, no new initiatives., now Theirs" + ); + assert!(focused(cx, handle, "conflict-reapply")); + click(cx, handle, "conflict-discard"); + assert!(!exists(cx, handle, "conflict")); + assert_eq!(form_value(cx, &manage, "notes"), ""); + assert_eq!(form_value(cx, &manage, "goal"), "Theirs"); + assert_eq!(form_state(cx, &manage), (None, Some(6), false)); + // The next edit saves on the version read back. + set_input(cx, handle, &manage, "notes", "again"); + click(cx, handle, "project-save"); + settle(cx, handle, &view).await; + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), 2); + assert_eq!(writes[1].precondition, Precondition::IfMatch(6)); + assert_eq!(backend.row("gh-x")["notes"], json!("again")); +} + +#[gpui_kit::test] +async fn a_malformed_reread_says_so_and_nothing_is_sent_until_save(cx: &mut TestAppContext) { + let folder = Folder::new(); + let backend = FakeProjects::new(&["read", "write:projects"]); + let (handle, view, manage) = open_with(cx, backend.clone(), &folder); + open_dialog(cx, handle, &view).await; + edit(cx, handle, &manage, "gh-x"); + set_input(cx, handle, &manage, "notes", "mine"); + // No project object, and a bare row, which isn't pmd's answer either. + let answers = [json!({"items": []}), project("gh-x", Some(3), true, "Theirs")]; + for (round, answer) in answers.into_iter().enumerate() { + backend.answer_next_read(answer); + backend.fail_next(WriteError::StaleVersion, |_| {}); + click(cx, handle, "project-save"); + settle(cx, handle, &view).await; + frame(cx, handle); + assert!(!exists(cx, handle, "conflict"), "round {round}"); + let failed = "Changed elsewhere, and reloading it failed: pmd returned malformed data."; + assert_eq!(text_of(cx, handle, "dialog-message"), failed); + assert_eq!(form_state(cx, &manage), (Some(failed.to_owned()), Some(2), true)); + assert_eq!(form_value(cx, &manage, "notes"), "mine", "the edit stays"); + let writes = backend.writes.lock().unwrap().clone(); + assert_eq!(writes.len(), round + 1, "one write per Save, never retried"); + assert!(writes.iter().all(|w| w.precondition == Precondition::IfMatch(2))); + assert_eq!( + backend.reads.load(Ordering::SeqCst), + round + 1, + "one re-read per conflict" + ); + } +} + #[gpui_kit::test] async fn a_risk_level_pick_writes_only_on_save_and_the_list_shows_it(cx: &mut TestAppContext) { let folder = Folder::new(); @@ -728,6 +824,10 @@ async fn a_create_that_timed_out_but_landed_becomes_an_edit_without_a_second_cre !exists(cx, handle, "project-field-repo"), "the stored project is edited now" ); + // The read back found it, so no "checking for the row failed". + assert!(!exists(cx, handle, "dialog-message")); + assert_eq!(form_state(cx, &manage), (None, Some(2), false)); + assert_eq!(backend.reads.load(Ordering::SeqCst), 1); press(cx, handle, "ctrl-enter"); assert_eq!(backend.writes.lock().unwrap().len(), 1, "never a second create"); click(cx, handle, "projects-back"); diff --git a/crates/dashboard-core/src/projects.rs b/crates/dashboard-core/src/projects.rs index a7475a0..7781fc0 100644 --- a/crates/dashboard-core/src/projects.rs +++ b/crates/dashboard-core/src/projects.rs @@ -125,6 +125,16 @@ impl Project { .collect() } + /// One project from `GET /v1/projects/{slug}`, which pmd answers as `{"project": {...}, "items": [...]}` (#94). + /// The open items aren't needed here. An answer without a `project` object is malformed, even a bare project row. + pub fn one_from_json(value: &Value) -> Result { + value + .get("project") + .filter(|project| project.is_object()) + .ok_or(WriteError::Malformed) + .and_then(Self::from_json) + } + pub fn path(&self) -> String { path(&self.slug) } diff --git a/crates/dashboard-core/tests/fixtures/pmd-project-codexbar-ios-30bf244.json b/crates/dashboard-core/tests/fixtures/pmd-project-codexbar-ios-30bf244.json new file mode 100644 index 0000000..058b912 --- /dev/null +++ b/crates/dashboard-core/tests/fixtures/pmd-project-codexbar-ios-30bf244.json @@ -0,0 +1,103 @@ +{ + "items": [ + { + "assignment": null, + "author": "HemSoft", + "ghState": "open", + "ghUpdatedAt": "2026-10-21T11:30:00Z", + "id": "ID01", + "kind": "pr", + "labels": [], + "links": [ + { + "kind": "closes", + "to": "ID02" + } + ], + "number": 3, + "priority": 0, + "priorityOverride": null, + "project": "codexbar-ios", + "rank": 0, + "status": "ready", + "statusReason": null, + "statusSource": "auto", + "title": "codexbar-ios pr 3", + "url": "https://github.com/hemsoft-dev/codexbar-ios/issues/3", + "version": 1, + "waitingReason": null, + "waitingSince": null + }, + { + "assignment": null, + "author": "HemSoft", + "ghState": "open", + "ghUpdatedAt": "2026-10-21T11:00:00Z", + "id": "ID02", + "kind": "issue", + "labels": [], + "links": [ + { + "by": "ID01", + "kind": "closes" + } + ], + "number": 1, + "priority": 0, + "priorityOverride": null, + "project": "codexbar-ios", + "rank": 0, + "status": "ready", + "statusReason": null, + "statusSource": "auto", + "title": "codexbar-ios issue 1", + "url": "https://github.com/hemsoft-dev/codexbar-ios/issues/1", + "version": 2, + "waitingReason": null, + "waitingSince": null + }, + { + "assignment": null, + "author": "HemSoft", + "ghState": "open", + "ghUpdatedAt": "2026-10-21T10:00:00Z", + "id": "ID03", + "kind": "issue", + "labels": [ + "waiting-on-franz" + ], + "links": [], + "number": 2, + "priority": 0, + "priorityOverride": null, + "project": "codexbar-ios", + "rank": 0, + "status": "waiting_on_franz", + "statusReason": null, + "statusSource": "auto", + "title": "codexbar-ios issue 2", + "url": "https://github.com/hemsoft-dev/codexbar-ios/issues/2", + "version": 1, + "waitingReason": "label:waiting-on-franz", + "waitingSince": "\u003ctime\u003e" + } + ], + "project": { + "counts": { + "openIssues": 2, + "openPrs": 1 + }, + "goal": "Goal of codexbar-ios", + "merged24h": 0, + "merged7d": 0, + "notes": "", + "priority": 0, + "repo": "hemsoft-dev/codexbar-ios", + "requires": [], + "riskLevel": 5, + "slug": "codexbar-ios", + "source": "portfolio", + "tracked": true, + "version": 1 + } +} diff --git a/crates/dashboard-core/tests/projects.rs b/crates/dashboard-core/tests/projects.rs index 638f24c..79a1d24 100644 --- a/crates/dashboard-core/tests/projects.rs +++ b/crates/dashboard-core/tests/projects.rs @@ -7,7 +7,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use dashboard_core::config::Config; use dashboard_core::edit::{Form, Next}; use dashboard_core::projects::{self, FIELDS, LinkError, Project}; -use dashboard_core::write::{Method, Precondition, WriteBackend, WriteError, WriteRequest}; +use dashboard_core::write::{CreateCheck, Method, Precondition, WriteBackend, WriteError, WriteRequest}; use serde_json::{Value, json}; #[path = "support/stub.rs"] @@ -326,6 +326,144 @@ fn a_risk_level_changed_elsewhere_is_a_difference_and_pmds_range_error_lands_on_ assert_eq!(stub.requests()[0].header("if-match"), Some("4")); } +/// pmd's `GET /v1/projects/{slug}` answer, from project-manager's `internal/api/testdata/project.golden.json` at +/// 30bf244 (#94). +const PMD_PROJECT: &str = include_str!("fixtures/pmd-project-codexbar-ios-30bf244.json"); + +#[test] +fn a_single_project_read_takes_pmds_project_object_and_nothing_else() { + let answer: Value = serde_json::from_str(PMD_PROJECT).unwrap(); + let one = Project::one_from_json(&answer).unwrap(); + assert_eq!( + ( + one.slug.as_str(), + one.repo.as_str(), + one.priority, + one.risk_level, + one.version + ), + ("codexbar-ios", "hemsoft-dev/codexbar-ios", Some(0), Some(5), 1) + ); + assert_eq!( + Project::from_json(&answer), + Err(WriteError::Malformed), + "the answer isn't a row" + ); + let malformed = [ + json!({"items": []}), + project(json!({})), + json!({"project": null, "items": []}), + json!({"project": [project(json!({}))]}), + json!({"project": {"slug": "gh-x"}, "items": []}), + ]; + for answer in malformed { + assert_eq!(Project::one_from_json(&answer), Err(WriteError::Malformed), "{answer}"); + } +} + +/// pmd's answer to `GET /v1/projects/gh-x` with `project` in place of its project. +fn wrapped(project: Value) -> Reply { + Reply::json(200, json!({"project": project, "items": []})) +} + +#[test] +fn a_stale_version_rereads_pmds_wrapped_project_into_the_conflict() { + let dir = Dir::new(); + let puts = AtomicUsize::new(0); + let stub = Stub::new(move |request| match request.method.as_str() { + "PUT" if puts.fetch_add(1, Ordering::SeqCst) == 0 => { + Reply::error(409, "stale_version", "If-Match 3 is not the current version 4") + } + "PUT" => Reply::json(200, project(json!({"goal": "Mine", "version": 5}))), + _ => wrapped(project(json!({"goal": "Theirs", "version": 4}))), + }); + let writer = dir.writer(&stub); + let gh_x = Project::from_json(&project(json!({}))).unwrap(); + let mut edit = Form::edit(gh_x.row(), &FIELDS); + edit.set("goal", "Mine"); + let put = |edit: &mut Form| { + let (slug, body) = projects::body(edit.values()).unwrap(); + let precondition = edit.begin_save().unwrap(); + writer.write(&WriteRequest { + method: Method::Put, + path: projects::path(&slug), + precondition, + body, + }) + }; + let error = put(&mut edit).unwrap_err(); + assert_eq!(edit.failed(&error), Next::Reread); + let theirs = writer + .read(&gh_x.path()) + .and_then(|value| Project::one_from_json(&value)) + .unwrap(); + edit.conflict(theirs.row()); + assert_eq!( + edit.conflict_state().unwrap().differences(), + [("goal", "Mine", "Theirs")] + ); + edit.reapply(); + let saved = Project::from_json(&put(&mut edit).unwrap()).unwrap(); + edit.saved(saved.row()); + let requests = stub.requests(); + let seen: Vec<(&str, &str, Option<&str>)> = requests + .iter() + .map(|r| (r.method.as_str(), r.path.as_str(), r.header("if-match"))) + .collect(); + assert_eq!( + seen, + [ + ("PUT", "/v1/projects/gh-x", Some("3")), + ("GET", "/v1/projects/gh-x", None), + ("PUT", "/v1/projects/gh-x", Some("4")), + ] + ); + assert_eq!(requests[1].token(), stub::READ, "the re-read uses the read token"); + assert_eq!(edit.version(), Some(5)); +} + +#[test] +fn a_create_that_timed_out_reads_back_the_project_pmd_stored() { + let dir = Dir::new(); + let stub = Stub::new(|request| match request.method.as_str() { + "PUT" => { + let mut reply = Reply::json(201, project(json!({"slug": "yahtzee", "repo": "hemsoft-dev/yahtzee"}))); + reply.delay = std::time::Duration::from_millis(1500); + reply + } + _ => wrapped(project( + json!({"slug": "yahtzee", "repo": "hemsoft-dev/yahtzee", "version": 1}), + )), + }); + let mut writer = dir.writer(&stub); + writer.timeout = std::time::Duration::from_millis(500); + let mut values = new_project(); + values.insert("slug".into(), "yahtzee".into()); + values.insert("repo".into(), "hemsoft-dev/yahtzee".into()); + let (slug, body) = projects::body(&values).unwrap(); + let error = writer + .write(&WriteRequest { + method: Method::Put, + path: projects::path(&slug), + precondition: Precondition::IfNoneMatch, + body, + }) + .unwrap_err(); + assert_eq!( + error, + WriteError::Timeout { + create: Some(CreateCheck::Exists) + } + ); + let stored = writer + .read(&projects::path(&slug)) + .and_then(|value| Project::one_from_json(&value)) + .unwrap(); + assert_eq!((stored.slug.as_str(), stored.version), ("yahtzee", 1)); + let methods: Vec = stub.requests().iter().map(|r| r.method.clone()).collect(); + assert_eq!(methods, ["PUT", "GET", "GET"], "one create, never resent"); +} + /// A temporary folder: a config dir and checkouts with a given origin. struct Folder(PathBuf); impl Folder {