Skip to content

[#267] Indexing recovery UI for failed publish#282

Merged
realproject7 merged 3 commits intomainfrom
task/267-indexing-recovery-ui
Mar 18, 2026
Merged

[#267] Indexing recovery UI for failed publish#282
realproject7 merged 3 commits intomainfrom
task/267-indexing-recovery-ui

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • New usePublishIntent hook: localStorage-based intent that persists content, metadata, tx hash, and indexer route across page reloads
  • Updated usePublish to accept intent lifecycle callbacks (onIntentSave, onTxConfirmed, onIndexed) — user-rejected transactions clear intent immediately, all other errors preserve it for recovery
  • New RecoveryBanner component: shows on create/chain pages when a pending intent exists, with retry (re-sends indexer POST) and dismiss options
  • Wired into both /create (storyline) and /chain (plot chaining) pages

Key design decisions

  • Error classification in usePublish.ts: Only user-rejected tx clears the intent. Network errors, receipt poll failures, and indexer failures all preserve the intent for recovery (addresses T2a feedback on PR [#267] Indexing recovery UI for failed publish #280)
  • Lazy useState init instead of useEffect for loading pending intent (avoids React strict-mode lint rule)
  • 409 from indexer treated as idempotent success (intent cleared)
  • Max 5 retries with warning after exhausted; dismiss available at any time

Test plan

  • Create a storyline — verify intent is saved to localStorage before wallet popup
  • Reject wallet popup — verify intent is cleared (no false recovery on reload)
  • Simulate indexer failure (e.g., disconnect network after tx confirms) — verify recovery banner on page reload
  • Click "Retry Indexing" — verify indexer POST is re-sent with all metadata
  • Click "Dismiss" — verify intent is cleared
  • Repeat above for plot chaining (/chain)
  • npm run typecheck passes
  • npm run lint passes (no new warnings)

Fixes #267

🤖 Generated with Claude Code

localStorage-based intent persists content + metadata before wallet
confirmation. If the indexer call fails after a successful on-chain tx,
a recovery banner appears on next page load with retry/dismiss options.

User-rejected transactions clear the intent immediately (no false
recovery). 409/duplicate responses treated as idempotent success.

Fixes #267

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES

Summary

The recovery flow is close, but it misses one of the issue's required failure modes: once the wallet submits a transaction, a later receipt-poll failure should still leave enough state for recovery.

Findings

  • [high] Receipt-poll failures are not recoverable because txHash is only persisted after waitForTransactionReceipt succeeds.
    • File: src/hooks/usePublish.ts:121
    • Suggestion: Persist the transaction hash immediately after writeContractAsync returns it, before awaiting receipt confirmation. Right now, if the wallet broadcast succeeds and receipt polling fails, the stored intent still has txHash: null, loadPendingIntent() filters it out, and the recovery banner never appears. That violates issue #267's explicit requirement for the "tx sent but receipt poll failed" path.

Decision

Requesting changes because this leaves a permanent gap in the recovery mechanism for one of the documented acceptance paths.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

T2b Review — REQUEST CHANGES

Overall approach is solid. The intent lifecycle and error classification address T2a's PR #280 feedback well. Three issues to fix:

1. RecoveryBanner.tsx:25-28 — Missing try/finally in handleRetry

If onRetry throws (not just returns { success: false }), setRetrying is never set back to false, permanently disabling both buttons.

async function handleRetry() {
  setRetrying(true);
  try {
    await onRetry();
  } finally {
    setRetrying(false);
  }
}

2. useChainPlot.ts:41 — Unstable object reference in dependency array

intentCallbacks is an object literal created on every render of the parent component, so chainPlot is recreated every render despite all individual callbacks being stable (useCallback). Either destructure into individual params or useMemo the callbacks object in the parent.

3. usePublish.ts:147onIndexed?.() to clear intent on user rejection is semantically misleading

Calling onIndexed when no indexing happened makes the callback contract confusing. Consider adding a dedicated onIntentClear callback, or at minimum rename the existing callback to something like onComplete that covers both success and rejection-clear cases. Current code works but will confuse future readers.


Items 1 and 2 are bugs. Item 3 is a strong suggestion — won't block on it if you have a reason to keep it.

…lling

Fixes T2a review: if receipt polling fails after wallet broadcasts,
the intent now has the tx hash and recovery banner will appear.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Follow-up blocking finding after the receipt-poll fix:

  • [high] src/hooks/usePublish.ts:132 still treats indexer HTTP failures as success because the fetch() response is ignored. A 500/400 response will fall through to onIndexed?.(), clear the saved intent, and set state to published, so the recovery banner never appears. The issue explicitly requires 500/network failures to preserve recovery state, with only 409 treated as idempotent success. Please gate success on response.ok || response.status === 409 and keep the intent on other statuses.

Only clear intent on 2xx or 409 (duplicate). Non-success responses
now throw, preserving the intent for recovery.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

Verdict: APPROVE

Summary

The recovery flow now covers the required failure modes: tx hash is persisted immediately after broadcast, and indexer HTTP failures preserve the saved intent unless the response is successful or a 409.

Findings

  • No blocking findings from this review pass.

Decision

Approving from my side. The implementation now matches the issue's recovery requirements; GitHub checks were still pending at the time of review.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

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

T2b Re-review — APPROVED

T2a's findings (tx hash persistence timing, indexer response status) are both fixed correctly:

  • onTxConfirmed now fires after writeContractAsync and before waitForTransactionReceipt — recovery works even if receipt polling fails
  • Indexer response is checked: only 2xx/409 clears intent, other statuses throw and preserve recovery state

My two items from the first review remain but are non-blocking:

  1. RecoveryBanner.tsx handleRetry missing try/finally — attemptRetry has internal try/catch so won't throw in practice
  2. useChainPlot.ts unstable intentCallbacks object in dep array — perf concern only, callbacks are lightweight

Ship it.

@realproject7 realproject7 merged commit 2296f8f into main Mar 18, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Front-end recovery UI for failed indexing after successful on-chain tx

2 participants