Repository navigation
feat(merge_requests): guard regular private review saves - #607
Conversation
Share discussion write reservations while confirming fresh global MR identity and preserving exact private Markdown. Block uncertain save replay until complete read-only inspection, retaining settlement barriers across same-account refresh and client replacement. Signed-off-by: sjungwon03 <sjungwon03@gmail.com>
Signed-off-by: sjungwon03 <sjungwon03@gmail.com>
sjungwon03-ai
left a comment
There was a problem hiding this comment.
Review of commit 78f8e5d: Reviewed final source, read lifecycle/reservation interactions, private reader revisions, 65 new test-first regressions and documentation. Regular saves preserve exact Markdown, derive the positive global identity from fresh authenticated detail separately from IID, validate captured ownership and response body/identity, and perform one create only. Duplicate saves and public discussion operations are blocked during preflight/write/complete inspection. Account/client/repository/detail replacement and view checks isolate late private outcomes and cancel obsolete waits. Unconfirmed/failed dispatched saves retain the inspection requirement across refresh and same-account client/wrapper changes; recovery waits for actual dispatched request settlement before staging all private pages. Author/global identity, positive unique IDs and advancing cursors are checked through empty pages; obsolete or malformed reads never return a partial list or dispatch another page. Success refreshes matching private query variants without public-comment fallback or analytics. Local gates passed 6,154 tests, clean analysis and formatting; all four required CI jobs passed on this exact head. No blocking findings. Composer UI, positioned/reply save orchestration, editing/deletion and publication remain follow-ups. In-memory uncertainty, non-atomic offset reads and possible server processing after a client timeout cannot guarantee exactly-once creation; no live GitLab/device/store validation is claimed.
Summary
Regular private review-note creation had an API but no orchestration that could safely share discussion writes or recover from an uncertain save. This adds
savePendingNoteandinspectPendingNotesto the existing MR discussion controller. Exact nonblank Markdown is saved once using fresh authenticated detail and a positive global MR ID separately from the route IID; confirmed current-session success refreshes every private-reader variant for this route only.Saves and complete inspection share the controller's comment/reply/resolution/suggestion/pagination reservation. Repository/account/client/detail replacement cancels obsolete waits and suppresses late private outcomes. Failed or unconfirmed dispatched saves block retry across controller refresh and same-account client/repository replacement. Explicit read-only recovery waits for any dispatched write to settle, then stages all private pages, including empty cursors, validating identity/ownership/uniqueness and advancing cursors. It never replays a create or infers that identical text identifies the failed attempt.
Closes #606
Validation
Checklist
Notes for reviewers
The reservation is the existing discussion controller's; approval/merge actions use their separate controller. Started read failures are handled immediately even while another repository waits. Cancellation is bridged once per command and local wait handlers are released on completion. Obsolete writes are not undone; same-account recovery waits for their actual request futures before reading.
Uncertainty is in memory only and cannot promise durable recovery across a process/container restart. Even a settled client timeout can leave server processing ongoing, and offset inspection is not atomic or exactly-once creation. A future composer must display the complete inspection snapshot before offering manual retry and check its current view before rendering results or clearing input. No composer UI, positioned/reply/commit save orchestration, editing/deletion, publication, automatic deduplication, analytics or disk cache is shipped here. MW-07 stays in progress.