Skip to content

fix: read pmd's wrapped single-project answer in the Projects re-reads - #95

Merged
HemSoft merged 1 commit into
mainfrom
fix/issue-94-projects-wrapped-reread
Nov 7, 2026
Merged

HemSoft merged 1 commit into
mainfrom
fix/issue-94-projects-wrapped-reread

Conversation

@HemSoft

@HemSoft HemSoft commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Risk level: 2/10 (project-dashboard)
Worked on by: claude-opus-5-5 on laptop, for Chief dispatch 17533a35

Summary

pmd answers GET /v1/projects/{slug} as {"project": {...}, "items": [...]}. Manage > Projects parsed that whole answer as a project row, so the re-read after 409 stale_version, and after a create that timed out but landed, always failed as malformed. The Projects test fake answered with a bare row, so the tests never saw pmd's shape.

  • dashboard-core/src/projects.rs: Project::one_from_json takes the project object out of the answer. An answer without one is malformed, a bare row included.
  • manage/projects.rs:640: the one re-read that both paths share now calls it. The PUT answer, which pmd sends as a bare row, still goes through Project::from_json (line 572).
  • projects_tests.rs: FakeProjects::read answers in pmd's wrapped shape, can be told to send one other answer, and counts reads.
  • tests/fixtures/pmd-project-codexbar-ios-30bf244.json: project-manager's internal/api/testdata/project.golden.json at 30bf244, byte for byte (sha256 6a5dcc4e…c55c744).

Not changed: write semantics, If-Match/If-None-Match, no automatic retry, the error messages, Work items and Fleet (pmd sends their single rows bare), and what Reapply sends.

Acceptance Criteria

  • With pmd's wrapped answer, a stale-version save shows the conflict panel. Reapply sends one PUT with the re-read version, and Discard loads pmd's values. Evidence: changed_elsewhere_shows_both_values_and_reapplies_on_the_new_version (2 writes, the second If-Match: 6) and changed_elsewhere_can_discard_mine_for_the_reread_values (goal and notes from pmd, version 6, not dirty; the next save sends If-Match: 6). Also a_risk_level_changed_elsewhere_shows_both_values_and_reapplies, both a_priority_changed_elsewhere_* tests, and core a_stale_version_rereads_pmds_wrapped_project_into_the_conflict over the loopback stub (PUT If-Match 3, GET with the read token, PUT If-Match 4).
  • A create that times out and whose row exists ends with the stored project in the editor and the list, with no "checking for the row failed". Evidence: a_create_that_timed_out_but_landed_becomes_an_edit_without_a_second_create (title yahtzee (hemsoft-dev/yahtzee), no dialog message, version 2, one read, one write, listed after Back) and core a_create_that_timed_out_reads_back_the_project_pmd_stored (PUT, GET, GET; never resent).
  • A malformed re-read still fails the same way, and nothing is retried. Evidence: a_malformed_reread_says_so_and_nothing_is_sent_until_save ({"items": []}, then a bare row: each shows "Changed elsewhere, and reloading it failed: pmd returned malformed data.", keeps the edit and the stale version 2, one write per Save and one read per conflict) and core a_single_project_read_takes_pmds_project_object_and_nothing_else (pmd's golden parses; five malformed shapes are refused).
  • The Projects fake answers read("v1/projects/{slug}") in pmd's wrapped shape.
  • Work items and Fleet re-reads behave as before. Their bare-row fakes and conflict tests are unchanged and pass: work_items_tests::changed_elsewhere_shows_both_values_then_reapplies_on_the_new_version, work_items_tests::changed_elsewhere_can_discard_mine and the Fleet conflict tests in fleet_tests.rs.

Definition of Done

  • Parsing in dashboard-core with unit tests against pmd's golden and malformed shapes.
  • Loopback-stub core tests for the stale-version conflict and the timed-out create.
  • Headless UI tests through the real GPUI seam fail on the base caller and pass at this head (below).
  • fmt, clippy, workspace tests and standalone core pass on Windows locally (below).
  • cargo deny --locked check advisories: advisories ok locally, and CI's advisories job passed.
  • All five required CI checks pass for e604380: advisories, core (ubuntu-latest), core (windows-latest), rust (ubuntu-latest) and rust (windows-latest) (run 42350135376).
  • Current-head SFL review: Round 1 PASS, zero findings on e604380b909d01aed99b27b7712e3f65cff1ba74.

Validation

Local runs on Windows 11 (Laptop), not CI.

RED, at the real app seam: this head's tests and fake, with only manage/projects.rs:640 put back to the base Project::from_json. cargo test -p dashboard-app --locked projects_tests: 16 passed, 7 failed.

  • changed_elsewhere_shows_both_values_and_reapplies_on_the_new_version, a_risk_level_changed_elsewhere_shows_both_values_and_reapplies and changed_elsewhere_can_discard_mine_for_the_reread_values: no conflict-* element.
  • a_priority_changed_elsewhere_moves_the_row_after_the_conflict and a_priority_changed_elsewhere_keeps_the_project_selected_after_the_conflict: no conflict-discard.
  • a_create_that_timed_out_but_landed_becomes_an_edit_without_a_second_create: the title stays New project.
  • a_malformed_reread_says_so_and_nothing_is_sent_until_save: round 0 ({"items": []}) passes there too, as a control. Round 1 fails because the base accepts a bare row.

The core tests call the new one_from_json, so they don't build on the base.

GREEN at this head:

  • cargo test -p dashboard-app --locked projects_tests: 23 passed.
  • cargo fmt --all --check: pass.
  • cargo clippy --workspace --all-targets --locked -- -D warnings: pass.
  • cargo test --workspace --locked: pass. dashboard-app 209 passed (captures are compiled out on Windows); every dashboard-core suite passed, projects 20.
  • Standalone core with its own clean CARGO_TARGET_DIR: cargo check -p dashboard-core --locked and cargo test -p dashboard-core --locked: pass.
  • cargo deny --locked check advisories (cargo-deny 0.20.2): advisories ok.

Screenshots

Rendered offscreen on Linux by the required rust (ubuntu-latest) job of CI run 42350135376 at this head e604380, step Screenshots: CAPTURE_DIR="$GITHUB_WORKSPACE/captures" cargo test -p dashboard-app --locked captures -- --ignored (19 passed, including captures_projects_reread). They come from that run's captures artifact, whose head_sha is e604380b909d01aed99b27b7712e3f65cff1ba74. Default 1100×760 window and theme. captures_projects_reread saves each frame and then checks it: the conflict's differences, one PUT with If-Match: 6, the stored project and the list.

Stale version, wrapped re-read. gh-x is saved at P1 while someone else changed its goal (version 6). The conflict panel shows Yours next to Now in pmd, and Reapply mine has focus.

e604380: Projects conflict after a wrapped re-read, goal and priority shown as Yours and Now in pmd

After Reapply mine. One PUT with If-Match: 6; gh-x is saved at P1, version 7. Reapply sends every form value, so the goal is the user's again. That is existing behaviour and out of this issue's scope.

e604380: gh-x saved at P1 after Reapply mine on the re-read version

A create that timed out but landed. The read back finds yahtzee, and the editor opens as the stored project (P3, risk 5). There is no "checking for the row failed" message and no second create.

e604380: yahtzee opened as the stored project after the create timed out

e604380: the Projects list with yahtzee at P3 after Back

No recording: GPUI's headless renderer gives still frames, and opening a desktop window for a screen recording was out of bounds for this run.

Risks and deferred work

  • one_from_json refuses a bare row. If a future pmd answered GET /v1/projects/{slug} without the wrapper, re-reads would fail visibly with the existing "malformed data" message, as they do today.
  • Unchanged, and out of this issue's scope: Reapply sends every form value, including fields the user didn't touch (edit.rs tests pin this), and fix: keep a Work item selected when its save leaves it past the first page #93's deferred Work items items.

Closes #94

🤖 Generated with Claude Code

#94)

pmd answers GET /v1/projects/{slug} as {"project": {...}, "items": [...]}.
The Projects dialog parsed the whole answer as a row, so the re-read after
409 stale_version and after a timed-out create always failed as malformed.
Project::one_from_json takes the project object and refuses anything else.
The Projects test fake now answers in pmd's shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@HemSoft HemSoft added bug Something isn't working model:claude-opus-5.5 PR authored by claude-opus-5.5 labels Nov 7, 2026
@HemSoft

HemSoft commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

@sfl review

@hemsoft-sfl-reviewer hemsoft-sfl-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Verdict: PASS • Round 1

Confidence: 9/10

Review done by claude-opus-5-5 (same family: same-family: codex gated: codex Weekly 167% SUS 60 🔴 pause, defer)

The PR adds Project::one_from_json, which unwraps pmd's {"project": {...}, "items": [...]} answer to GET /v1/projects/{slug}. The Projects dialog's re-read (after 409 stale_version and after a create that timed out but landed) now uses it, the test fake answers in pmd's shape, and the PR adds core, UI and capture tests. I found no evidenced defect: the wrapped shape matches pmd's handler and golden file at 30bf244, and the change is limited to the one re-read caller.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 1m 51s
Token count: Unavailable

Pass 🔴 Critical 🟠 High 🟡 Medium ⚪ Low
🔒 Security 0 0 0 0
✅ Accuracy 0 0 0 0
🧰 Quality & Maintainability 0 0 0 0
Total 0 0 0 0

Key Findings

No material defects found.

Coverage

  • Files reviewed:
    • crates/dashboard-core/src/projects.rs
    • crates/dashboard-app/src/manage/projects.rs
    • crates/dashboard-app/src/projects_tests.rs
    • crates/dashboard-app/src/captures.rs
    • crates/dashboard-core/tests/projects.rs
    • crates/dashboard-core/tests/fixtures/pmd-project-codexbar-ios-30bf244.json
  • Context read:
    • AGENTS.md
    • crates/dashboard-core/src/write.rs
    • crates/dashboard-core/src/edit.rs
    • crates/dashboard-app/src/main.rs
    • crates/dashboard-app/src/manage.rs
    • crates/dashboard-app/src/manage/work_items.rs
    • crates/dashboard-core/src/pmd.rs
    • crates/dashboard-core/tests/support/stub.rs
    • hemsoft-dev/project-manager internal/api/api.go@​30bf244
    • hemsoft-dev/project-manager internal/api/projects.go@​30bf244
    • hemsoft-dev/project-manager internal/api/testdata/project.golden.json@​30bf244
  • Verified:
    • The compare's base_commit.sha is fa8ffb7af7a4b1310e0c69e9247655713351f210, merge_base_commit.sha matches it, its last commit is e604380b909d01aed99b27b7712e3f65cff1ba74 and it is ahead_by 1. PR #95's head and base match the reserved SHAs.
    • pmd's API.project handler (api.go:443) writes {"project": p, "items": items} for GET /v1/projects/{slug}, which confirms the wrapped shape that one_from_json expects.
    • The new fixture is byte-identical to pmd's internal/api/testdata/project.golden.json at 30bf244.
    • one_from_json returns Malformed when project is missing, null, an array or not an object, and also for a bare row; otherwise it delegates to the unchanged from_json, so the required slug, repo, tracked and version checks still apply.
    • manage/projects.rs:640 is the only caller that re-reads a single project; the PUT response parsing at :572 and the list parsing at :289 (list_from_json) are unchanged.
    • write.rs create-check (CreateCheck::Exists) only checks that the read succeeded and never parses the body, so the wrapped answer has no effect on it.
    • On a malformed re-read, the stale-version branch calls reread_failed, which keeps the user's edits and resends nothing. The create branch falls back to Timeout{Unknown} plus a list reload. Both paths are unchanged.
    • The symbols the tests use (editor_form, visible_slugs, conflict_state, differences, is_dirty, version, message, Form::value, Folder::config_dir, FakeProjects::new, Project: PartialEq+Debug) exist at the head SHA with the needed visibility. captures and projects_tests are both cfg(test).
    • Each fake read increments the counter once, and the fake's write never calls read, so the asserted read counts of 1 and round + 1 match the code paths.
    • The PR head's check runs advisories, core and rust (ubuntu-latest and windows-latest) completed with success.
  • Not verified:
    • Runtime behavior, including GPUI rendering and capture PNGs; execution was forbidden.
    • The stub's behavior on a delayed PUT is inferred from the test, not read in full in support/stub.rs.
    • pmd at any revision newer than 30bf244; the shape was checked only at the revision the fixture names.
  • Tests run: None. Reviewer execution is disabled.

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head e604380b909d01aed99b27b7712e3f65cff1ba74 • SFL v0

@HemSoft
HemSoft merged commit a6ce1c8 into main Nov 7, 2026
5 checks passed
@HemSoft
HemSoft deleted the fix/issue-94-projects-wrapped-reread branch November 7, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working model:claude-opus-5.5 PR authored by claude-opus-5.5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read pmd's wrapped single-project answer in the Projects re-reads

1 participant