MotirBuilding in public
MOTIR · moooon
onMotir
You’re viewing a public project. Anyone can view it — no account needed. Sign in to submit, upvote, or comment on requests.View-only — you can’t edit work items
MOTIR-153

bug-inline-status-revert-on-second-edit Issue list: inline status edit — the first item's status sometimes reverts after editing a second item

Done
Description

Type: bug · Parent: Epic 2 · Surface: issue list inline cell editing (Story 2.5, Subtask 2.5.5 — the status / assignee / priority editors) · Reported by: Yue, 2026-06-10.

Change the status of work item A inline on /issues, then change the status of a second work item B — intermittently, A's status flips back to its previous value after the second edit. "Sometimes": it does not reproduce on every attempt; editing the two items in quick succession appears to raise the hit rate, which smells like a timing race rather than a deterministic logic error.

Repro (intermittent): sign in as zhuyue@prodect.co / !QAZ1qaz, open /issues in List view, inline-edit item A's Status cell (e.g. To do → In progress), then promptly inline-edit item B's Status cell. Watch A's cell after B's edit settles — on a hit, A renders its OLD status again.

Scoped to DISPLAY-ONLY (Yue, 2026-06-10): the backend is correct. Yue verified the API persists the update — A's row holds the NEW status in the database while the list renders the OLD one. So this is a client-side stale-display race in the list UI, not a lost write: no data corruption, but trust-breaking (the user is shown state the system knows is wrong, until the next reload).

⚠️ Client mechanism NOT diagnosed — failing repro test FIRST (the reproduce-before-diagnosing rule; the filter check-mark lesson). This card records the SYMPTOM plus the backend-correct scoping above; WHICH client interleaving causes the stale render has not been verified, and the fix MUST begin with a red repro test — not a code-reading theory. The test still asserts the DB row alongside the cell (locking in the backend-correct fact and catching any regression to a lost write), but the red assertion is the rendered cell.

Investigation surface (hypotheses to test, NOT conclusions). Each inline cell editor in IssueInlineEdit.tsx keeps a local optimistic override and calls router.refresh() after its PATCH resolves. Two rapid edits put two PATCHes + two refreshes in flight: candidate mechanisms include (a) B's refresh payload being read/snapshotted before A's write is visible and re-rendering A from stale server props after A's override is gone, (b) refresh responses resolving out of order, (c) the override being cleared by ANY refresh completion rather than its own. A deterministic repro can interleave these orderings — component/integration test with controlled response ordering, or Playwright with route interception delaying the first PATCH/refresh. Also check whether the detail-page status control and the board column-menu transition share the pattern (fix once in the shared mechanic if so).

Acceptance criteria

  • A repro test exists that is RED on the pre-fix code (two rapid inline status edits with adversarial response ordering → the first cell renders the old status) and green after; it asserts the rendered cell, and confirms the DB row holds the new status throughout (the backend-correct fact, locked in against regression).
  • After the fix, A's cell shows its new status across B's edit and every refresh ordering — the rendered list converges to the persisted state without a manual reload.
  • The fix covers all three inline editors sharing the mechanic (status / assignee / priority) — asserted for at least status + assignee — and any other surface found to share it during investigation.
  • Single-edit behavior unchanged: the existing inline-edit tests stay green.

Context refs

  • app/(authed)/issues/_components/IssueInlineEdit.tsx — the per-cell override + router.refresh() mechanic (lines ~95-200)
  • app/(authed)/issues/_components/issueColumns.tsx, IssueListTable.tsx — how server props flow back into the cells
  • lib/services/workItemsService.ts (updateStatus) — the persisted-state side the repro test must assert
  • tests/components/issue-inline-edit.test.tsx — the existing suite the repro extends
  • notes.html (reproduce-before-diagnosing; the twice-wrong filter check-mark bug)

Re-opened (Yue, 2026-06-10) — PR #619 did not fix it

The merged fix (useConvergingOverride, PR #619) defended each cell against stale full-tree payloads but KEPT the refresh fan-out that creates them — revalidatePath('/issues') in the field actions plus router.refresh() per cell put up to four whole-page RSC snapshots in flight for two quick edits, and the defense only lives in mounted component state. Yue verified the revert still happens in the live app and set the correct contract: a successful action response IS the confirmation — call the endpoint, confirm the optimistic value when it returns, no whole-tree refresh on success. Re-fix in PR #640: actions no longer revalidate, cells no longer refresh on success (only the optimistic-concurrency STALE conflict still refreshes), so there are no payloads left to race. The detail page CoreFieldsPanel shares the refresh-on-success mechanic but is owned by open PR #633 — logged as a finding (#79) for a follow-up, not touched in #640.

Closed (2026-06-11): PR #640 merged. Full CI green including the inline-edit E2E. The PR also fixed a consequence the first commit introduced and the E2E caught: with no refresh, server props freeze, so a follow-up edit on the same row submitted a dead expectedUpdatedAt — the provider now keeps a per-row ledger of server-acknowledged updatedAt values and submissions send max(ledger, prop). Regression tests cover same-cell (reassign → unassign) and cross-cell (status → assignee) follow-ups against the real service.