FE-674: Resolve pending review rows#117
Conversation
e11e4ed to
5704a0b
Compare
54006af to
aff3024
Compare
aff3024 to
ead1ecf
Compare
PR SummaryMedium Risk Overview Updates the patch-list overlay’s Pending review section to include a per-row Resolve button that calls the new API, disables/shows “Resolving…” while in flight, and refreshes the open-needs query after success. Adds focused server and component tests covering resolve behavior, idempotence, error cases, and UI pending-state/section hiding, and updates planning/design docs to reflect the single-action V3.0 Resolve shape. Reviewed by Cursor Bugbot for commit 308215b. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: This PR completes side-chat V3.0 by adding an idempotent “resolve reconciliation need” endpoint and wiring a per-row Resolve action into the patch-list overlay, allowing users to clear open cascade needs end-to-end. Changes:
Technical Notes: The UI uses TanStack Query invalidation ( 🤖 Was this summary useful? React with 👍 or 👎 |
ead1ecf to
b1542b0
Compare
0a6b7d4 to
01be53b
Compare
b1542b0 to
ec73476
Compare
9b5db93 to
6bc0376
Compare
ec73476 to
0fe4440
Compare
6bc0376 to
fa2e56b
Compare
0fe4440 to
4f63bdd
Compare
4f63bdd to
cb5c12b
Compare
fa2e56b to
7bade11
Compare
…s V3.0) Closes invariant I112's fifth and final clause: resolutions transition open→resolved idempotently. Closes the V3.0 frontier item (FE-674). Server: - New POST /api/specifications/:id/reconciliation-needs/:needId/resolve (handler `handleResolveReconciliationNeed` in src/server/reconciliation-needs-route.ts). Idempotent — already-resolved needs return 200 with no state change. Returns 404 on missing need OR cross-spec mismatch; 400 on non-numeric ids. - New `db.getReconciliationNeed(needId)` for ownership validation. Client: - New `resolveReconciliationNeedRequest` in src/client/lib/edit-api.ts. - patch-list-overlay: each open need row now has a Resolve button. Mid-flight state per-row tracked via `resolvingNeedIds` set; button disabled while pending and shows "Resolving…" copy. On success, the reconciliation-needs query invalidates and the row drops out of the list. Tests: - 6 new cases in resolve-reconciliation-need-route.test.ts cover transition, idempotence, 404 missing, 404 cross-spec, 400 non-numeric needId/specId. - 4 new patch-list-overlay tests cover: Resolve button per row, resolveReconciliationNeedRequest invocation, button disabled while pending, section disappears when last need resolves. Design clarification: - SIDE_CHAT.md §9 V3.0 row updated to note the original three-action design (accept-on-target / edit-target / dismiss) collapses to a single Resolve action for V3.0 because the open→resolved transition is the same regardless of intent label. V3.1 reintroduces richer kinds via the agent. Plan reconciliation: - PLAN.md: V3.0 moves Active → Recently Completed. Continuous workspace promotes to Active. V3.1 (agent-grouped resolution) moves from Horizon to Next, gated on V3.0 walkthrough signal for A88 validation. - Track B dependencies updated to reflect V3.0 completed. Verified: npm run verify (1063 tests, 0 lint warnings). Watch: - A88 (Path 1 sufficiency without agent) is mechanically validated; full validation requires outer-loop walkthrough on dense graphs. - D139 / A88 / I113 references in SPEC.md still carry the post-merge numbering drift from concurrent FE-698/FE-674 merges; ln-sync cleanup remains opportunistic.
The async IIFE that runs resolveReconciliationNeedRequest had no catch block, only try/finally. Request failures would propagate out of the finally as an unhandled promise rejection — visible only in browser console as 'Uncaught (in promise)' noise and easy to miss. Add an explicit catch that logs with the need id so failures are attributable. The row stays in the list (no optimistic removal) so the user can retry; surfacing a toast is left for when a project-wide toast mechanism lands.
The handler did a read-then-write (`getReconciliationNeed` to check status='open', then `resolveReconciliationNeed`), which under concurrent requests could let two callers both see 'open' and both write `resolved_at` — repeated resolves were silently rewriting the audit timestamp. Move the `status='open'` predicate into the UPDATE's WHERE clause so the database enforces the transition; SQLite makes the UPDATE atomic, so only the first wins. Drop the now-redundant read-side guard in the route.
cb5c12b to
308215b
Compare

What
Adds the Resolve button on each pending review row plus the server endpoint behind it. Closes V3.0 end-to-end.
Stacked on #116.
Why
#115 and #116 wrote and surfaced needs; users still had no way to clear them.
How
POST /api/specifications/:id/reconciliation-needs/:needId/resolve— idempotent open → resolved; 404 on missing or cross-spec; 400 on bad ids.Out of scope
Test plan