fix(ui): preserve an in-flight retry flag in notifyApiFailure - #8754
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
notifyApiFailure replaced the whole notifier entry (retrying: false) on every call for an id, so a second failure notification arriving while a retry was already in flight reset the re-entrancy flag runRetryWithProgress relies on — letting a second Retry click fire a concurrent duplicate retry. Preserve an existing retrying:true instead of resetting it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-26 03:01:07 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Fixes #8676
Re-file of #8751 (closed on
validate-code: the UI workspace'seslint .includes theprettier/prettierrule, and the object literal + a test's inline type needed prettier's multiline formatting). This branch is prettier-clean.Root cause
notifyApiFailure(apps/loopover-ui/src/lib/api/request.ts) rebuilt the notifier entry from scratch on every call for an id —notifierState.set(id, { …, retrying: false }). MeanwhilerunRetryWithProgressguards a double-fired retry by re-entrancy:if (entry?.retrying) return;then mutates the same shared objectentry.retrying = true, cleared only in.finally().Because
notifyApiFailurereplaced rather than merged, any call for the same label while a retry was in flight (a second component sharing the label, a background poll, or the retry's own failing request) silently resetretryingtofalse, defeating the guard — so a second Retry click fired a concurrent duplicate retry.Fix
Preserve the current entry's in-flight flag:
retrying: prev?.retrying === true. All other fields (kind,status,lastNotifiedAt,repeatCount) still update exactly as before — a one-field merge, not a behavior change anywhere else.Tests
retryingtrue; the first Retry click invokesretryonce; a mid-retrynotifyApiFailurefor the same label then a second Retry click must be blocked —retrystays called once. Verified genuinely non-vacuous: reverting the one-field change makes this fail (retrycalled twice).repeatCount(toast description shows "2× in a row").Validation
@loopover/uitest: 2 passed;prettier --checkclean on both files;typecheckclean; committed as LFapps/**is outside Codecov'scoverage.include, so no patch percentage applies; branch rebased onto currentmain;git diff --checkclean