Fix multi-chain claim flow under AppKit connections (standalone Superfluid campaign widget)

[goodbounties-nanoclaw-agent]

Closes #164

Scope

Standalone superfluid-campaign-web app only, per requester sign-off. Redirect support for other environments (in-wallet embedded widget) is left as-is for now.

Changes

• Network pill / active chain: the widget now reads the real active chain from the wallet context (chainIdOverride) instead of always assuming Celo.
• Chain switching under AppKit: switchChainOverride follows AppKit's own useAppKitNetwork() state and calls switchNetwork() with the matching AppKitNetwork descriptor (looked up via the now-exported DEFAULT_APPKIT_NETWORKS). Falls back to AppKit's own network-selection modal (open({ view: 'Networks' })) when there's no direct descriptor or when the programmatic switch throws.
• Connect/disconnect labeling: the button now reads "Disconnect" for direct injected providers and "Network settings" when AppKit is enabled, via a new disconnectLabel override threaded through CampaignHeader, LeaderboardView, and WalletChip.
• Live updates after AppKit disconnect: the custom display now re-renders from AppKit's own account/network hooks, so it reflects a real disconnect without needing a page refresh.
• All-chain claim visibility: loadClaimablesByChain now fetches claim entitlement for every supported chain (Fuse, Celo, XDC), not just the active one — using the wallet-bound SDK for the active chain and the SDK's checkEntitlement({ chainOverride, publicClient }) for the rest. This behavior is shared by both AppKit and direct WalletConnect/injected connectors.
• Working "switch network" action: switching now calls through to switchChain(targetChainId) instead of being a no-op, and defaults to the first chain that actually has a claimable amount rather than a hardcoded chain.
• Declarative copy: claim status text now says "G$ Claim is still available on <chain>" when another chain has a pending claim, and the "have to wait a little longer" copy is shown only when every chain's claim for the day is already done.

Testing

• npx turbo run build --filter=@goodwidget/superfluid-campaign-web... — 6/6 packages build clean (includes a full tsc type-check for the app).
• npx turbo run lint --filter=@goodwidget/superfluid-campaign-web... — clean.
• No existing automated test suite covers this app; manual verification is recommended against a real AppKit-connected wallet with claims pending on more than one chain.

[Copilot]

Pull request overview

Note

Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.

Adds integrator-controlled wallet state and UX overrides across widgets, improving AppKit compatibility and multi-chain claim behavior.

Changes:

• Extend widget/provider props to accept address/chain overrides, a chain-switch override, and a configurable wallet-chip action label.
• Centralize chain switching in GoodWidgetProvider and update Citizen Claim adapter to use it + improve per-chain entitlement reads.
• Update Superfluid campaign UI to forward disconnect label and embedded claim widget wallet overrides; expose AppKit default networks for embedding.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
packages/superfluid-campaign-widget/src/widgetRuntimeContract.ts	Adds new integrator override props to Superfluid Campaign widget contract.
packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx	Makes wallet chip action label configurable.
packages/superfluid-campaign-widget/src/components/LeaderboardView.tsx	Threads disconnectLabel through to WalletChip.
packages/superfluid-campaign-widget/src/components/CampaignHeader.tsx	Threads disconnectLabel through to WalletChip.
packages/superfluid-campaign-widget/src/SuperfluidCampaignWidget.tsx	Forwards wallet overrides into embedded Citizen Claim widget and surfaces disconnect label.
packages/embed/src/appkitProvider.ts	Re-exports useAppKitNetwork and DEFAULT_APPKIT_NETWORKS for embed consumers.
packages/embed/src/DefaultAppKitProvider.tsx	Exports DEFAULT_APPKIT_NETWORKS.
packages/core/src/types.ts	Extends GoodWidgetProviderProps with wallet overrides and disconnect label.
packages/core/src/provider.tsx	Implements address/chain override logic + adds switchChain and disconnectLabel to wallet context.
packages/citizen-claim-widget/src/widgetRuntimeContract.ts	Adds wallet override props to Citizen Claim widget contract.
packages/citizen-claim-widget/src/adapter.ts	Uses provider switchChain and improves multi-chain entitlement reads.
packages/citizen-claim-widget/src/CitizenClaimWidget.tsx	Shelves UI inside provider to read live chainId and forwards wallet overrides.
apps/superfluid-campaign-web/src/App.tsx	Demonstrates AppKit-driven overrides (address/chain/switch + disconnect label).

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[L03TJ3]

gdpatchagent will follow up on review

[goodbounties-nanoclaw-agent]

Fixed in 00d029c, addressing 2 of Copilot round-7's suppressed low-confidence comments that I judged to be real, in-scope bugs:

1. adapter.ts loadClaimStatus — a non-custodial connection whose active chain hadn't reported yet (chainId still null right after connecting) was mapped straight into unsupported_chain, flashing "switch network" copy before the real chain resolved. Added an early branch that treats !isCustodialExecution && chainId === null as still-loading instead.
2. WalletChip.tsx — if disableWalletButton flipped to true while the menu was already open, the menu stayed open and its disconnect action stayed pressable. The menu render is now also gated on !disabled.

The other 2 suppressed comments from round 7 I'm explicitly leaving as-is (nice-to-have, not affecting end-user UX or correctness):

• createReadOnlyClientsForChain creating a fresh viem client per call rather than memoizing — a perf micro-optimization with no observable UX impact given how infrequently this path runs.
• The read-only walletClient's HTTP transport being theoretically reusable for send-style RPCs — defensive-only; there's no code path that actually invokes it that way.

Re-requesting Copilot review now.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (6)

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:39

• When disabled flips from false → true while the menu is open, the menu stops rendering but isMenuOpen stays true. If the chip later becomes enabled again, the menu will immediately reappear without a user click. Consider closing the menu state whenever disabled becomes true (e.g. set isMenuOpen to false in an effect or during the disabled transition).

  const [isMenuOpen, setIsMenuOpen] = useState(false)

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:43

• When disabled flips from false → true while the menu is open, the menu stops rendering but isMenuOpen stays true. If the chip later becomes enabled again, the menu will immediately reappear without a user click. Consider closing the menu state whenever disabled becomes true (e.g. set isMenuOpen to false in an effect or during the disabled transition).

    <XStack position="relative" alignItems="center" opacity={disabled ? 0.5 : 1}>

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:57

• When disabled flips from false → true while the menu is open, the menu stops rendering but isMenuOpen stays true. If the chip later becomes enabled again, the menu will immediately reappear without a user click. Consider closing the menu state whenever disabled becomes true (e.g. set isMenuOpen to false in an effect or during the disabled transition).

        cursor={disabled ? 'not-allowed' : 'pointer'}
        onPress={() => {
          if (disabled) return
          setDisconnectMessage(null)
          setIsMenuOpen((open) => !open)
        }}

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:70

• When disabled flips from false → true while the menu is open, the menu stops rendering but isMenuOpen stays true. If the chip later becomes enabled again, the menu will immediately reappear without a user click. Consider closing the menu state whenever disabled becomes true (e.g. set isMenuOpen to false in an effect or during the disabled transition).

      {isMenuOpen && !disabled && (

packages/core/src/provider.tsx:60

• Some connectors/providers surface EIP-1193 user rejection as code: '4001' (string) rather than 4001 (number). In that case, isUserRejectedSwitchChain would return false and switchChain may incorrectly fall back to switchChainOverride, potentially re-prompting the user right after rejection. Consider treating '4001' as a rejection as well.

function isUserRejectedSwitchChain(err: unknown): boolean {
  const code = (err as { code?: number | string } | undefined)?.code
  return code === 4001 || code === 'ACTION_REJECTED'
}

packages/citizen-claim-widget/src/adapter.ts:398

• SUPPORTED_CHAINS is sourced from citizenSdkCapabilities.chains, but createReadOnlyClientsForChain hard-depends on CHAIN_CONFIGS[targetChainId] having an RPC URL. If capabilities ever include a supported chain that isn’t present in CHAIN_CONFIGS (or lacks an RPC URL), claimable reads for that chain will be silently skipped (and status resolution can degrade to not_connected/missing data). Consider enforcing that CHAIN_CONFIGS fully covers SUPPORTED_CHAINS (e.g. assertion at init), or falling back to an existing public-client resolver for that chain.

      const chain = CHAIN_CONFIGS[targetChainId]
      const rpcUrl = chain?.rpcUrls.default.http[0]
      if (!chain || !rpcUrl) return null

[goodbounties-nanoclaw-agent]

Round 8 follow-up, fixed in e14d1a1:

1. App.tsx — chainIdOverride wasn't gated on isAccountResolved the way addressOverride already was, so a disconnected account could keep showing a stale previously-tracked chain. Now mirrors the address logic.
2. WalletChip.tsx — fixed the follow-up half of the earlier disabled-menu fix: isMenuOpen itself now resets (not just its render) when disabled turns on, so re-enabling the chip later doesn't reopen the menu without a click.
3. provider.tsx — isUserRejectedSwitchChain only matched the numeric rejection code 4001; some connectors surface it as the string '4001'. Now coerces with Number(code) before comparing.

Leaving the remaining suppressed nit (CHAIN_CONFIGS not being asserted to fully cover SUPPORTED_CHAINS) as a flagged nice-to-have rather than fixing now — it's a defensive assertion against future drift between this widget's chain config and the citizen SDK's own capability list, not a bug in current behavior (the two are in sync today). Happy to add the assertion if preferred.

Re-requesting Copilot review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (3)

packages/citizen-claim-widget/src/adapter.ts:575

• When SDK initialization fails for a supported chain (!sdk), setting status to 'not_connected' is misleading because an address is present and the wallet may be connected; it also prevents the UI from surfacing a meaningful failure mode. Prefer setting status to 'error' and setting a user-facing error message (or throwing a CitizenClaimAdapterError that ends up in humanReadableError) so the widget can explain that it couldn’t initialize RPC/SDK clients for the chain.

    // A personalized status read, but still address-only: no connected
    // account or passed-down provider is required, only the address itself.
    const sdk = createReadOnlySdkForChain(statusChainId)
    if (!sdk) {
      setStatus('not_connected')
      return
    }

packages/citizen-claim-widget/src/CitizenClaimWidget.tsx:444

• The unsupported_chain message and the “Ready to claim”/amount block can render at the same time (because the second condition is driven by claimablesByChain.length > 0). This produces conflicting status copy for an unsupported active chain. Make these branches mutually exclusive (e.g., exclude unsupported_chain from the second condition, or convert to an else if structure) so unsupported-chain messaging is the sole status UI in that state.

              {status === 'unsupported_chain' && (
                <>
                  <Text secondary>
                    {chainId
                      ? `Claiming isn't available on ${getChainName(chainId)}. Switch network to continue.`
                      : 'Switch to a supported network to claim daily G$'}
                  </Text>
                </>
              )}

              {(status === 'eligible' || status === 'claiming' || claimablesByChain.length > 0) && (
                <>
                  {/*
                    Declarative to claim status: an already_claimed wallet with
                    other chains still available must not read as "no claims
                    left" (the "Just a little longer" copy below is reserved for
                    when every chain has actually been claimed for the day).
                  */}
                  <Text secondary>
                    {status === 'already_claimed' && claimablesByChain.length === 1
                      ? `G$ Claim is still available on ${getChainName(claimablesByChain[0].chainId)}`
                      : status === 'already_claimed' && claimablesByChain.length > 1
                        ? 'G$ Claim is still available on other chains'
                        : 'Ready to claim'}
                  </Text>
                  {displayAmount && <TokenAmount token="G$" amount={displayAmount} size="xl" />}
                </>
              )}

packages/citizen-claim-widget/src/CitizenClaimWidget.tsx:353

• The comment says there is “no known claimable chain to target yet” and that this “falls back to Celo”, but the code actually prefers claimablesByChain[0]?.chainId when available. Update the comment to reflect the actual behavior (prefer first claimable chain, otherwise Celo), or change the logic to always prefer Celo if that’s the intended default.

          // The wallet is connected but on a chain citizen-sdk doesn't support at
          // all, so there is no known claimable chain to target yet — fall back
          // to Celo, the first preferred supported chain.
          await actions.switchChain?.(claimablesByChain[0]?.chainId ?? SupportedChains.CELO)

[goodbounties-nanoclaw-agent]

Round 9 came back with no new blocking comments, but fixed 2 of its 3 suppressed nits — both are real bugs, not polish (commit ed76c51):

1. CitizenClaimWidget.tsx — the unsupported_chain message block and the eligible/claiming/"Ready to claim" block could render at the same time whenever claimablesByChain wasn't empty (e.g. the active chain is unsupported but the wallet still has claimable UBI on another chain) — showing two contradictory status lines together. Made them mutually exclusive.
2. adapter.ts — loadClaimStatus set status to not_connected when the read-only SDK failed to initialize for an already-known, supported chain. That's misleading once an address is already known and connected; changed it to error with a clear message instead.

Also fixed a stale comment (switch_chain case) that no longer matched the code.

Re-requesting Copilot review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (4)

packages/citizen-claim-widget/src/adapter.ts:78

• CitizenClaimAdapterError is used with instanceof checks, but the constructor doesn’t set the prototype chain explicitly. In some TS/JS build targets (especially when downleveling), instanceof CitizenClaimAdapterError can fail for Error subclasses. Consider adding Object.setPrototypeOf(this, new.target.prototype) (or the class prototype) in the constructor to make the instanceof check reliable.

/** Resolves a supported chain id to its display name, falling back to the raw id. */
function getChainDisplayName(chainId: number): string {
  return CHAIN_CONFIGS[chainId]?.name ?? `Chain ${chainId}`
}

/**
 * Thrown for adapter-level failures whose message is already user-facing
 * (e.g. naming the specific chain an action cannot run on). humanReadableError
 * passes these through verbatim instead of remapping them to a generic string.
 */
class CitizenClaimAdapterError extends Error {
  constructor(message: string) {
    super(message)
    this.name = 'CitizenClaimAdapterError'
  }
}

packages/core/src/provider.tsx:156

• When only one override is provided (e.g. addressOverride but not chainIdOverride), the effect still subscribes to both events even though one handler becomes a no-op. This is minor overhead, but easy to avoid by conditionally registering only the listeners that can update state (e.g. only subscribe to accountsChanged when !hasAddressOverride, and only to chainChanged when !hasChainIdOverride).

  useEffect(() => {
    if (hasAddressOverride && hasChainIdOverride) return
    if (!resolvedProvider) return

    const handleAccountsChanged = (accounts: string[]) => {
      if (!hasAddressOverride) setTrackedAddress(accounts[0] ?? null)
    }
    const handleChainChanged = (newChainId: string) => {
      if (!hasChainIdOverride) setTrackedChainId(parseInt(newChainId, 16))
    }

    resolvedProvider.on('accountsChanged', handleAccountsChanged)
    resolvedProvider.on('chainChanged', handleChainChanged)

packages/core/src/provider.tsx:227

• If wallet_switchEthereumChain fails due to “method not supported” or a connector-specific failure and there’s no switchChainOverride, the code rethrows the raw provider error. That raw message often isn’t user-friendly and may leak low-level details into UI error surfaces. Consider normalizing non-user-rejection failures to a consistent, user-facing error (similar to SWITCH_CHAIN_UNAVAILABLE_ERROR) when no override is available, while still preserving the raw error for logging/debugging.

  const switchChain = useCallback(
    async (targetChainId: number) => {
      if (resolvedProvider) {
        try {
          await resolvedProvider.request({
            method: 'wallet_switchEthereumChain',
            params: [{ chainId: `0x${targetChainId.toString(16)}` }],
          })
          return
        } catch (err) {
          if (!switchChainOverride || isUserRejectedSwitchChain(err)) throw err
        }
      }
      if (!switchChainOverride) {
        throw new Error(SWITCH_CHAIN_UNAVAILABLE_ERROR)
      }
      await switchChainOverride(targetChainId)

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:50

• Applying opacity to the entire chip (including text) can reduce contrast below WCAG thresholds, especially on low-contrast themes, and can make the address hard to read. Consider using theme “disabled” tokens (e.g. muted text color/border color) or applying opacity only to non-text decoration while keeping text contrast compliant; also ensure the disabled state isn’t conveyed by opacity alone.

    <XStack position="relative" alignItems="center" opacity={disabled ? 0.5 : 1}>

[goodbounties-nanoclaw-agent]

Round 10 Copilot review — 0 non-suppressed comments, 4 suppressed (low-confidence) findings. Triage:

1. adapter.ts:78 (CitizenClaimAdapterError instanceof under downleveling) — recurrence of an item already assessed in an earlier round. This package builds with tsup target es2022 (native class/extends Error, no downleveling), so instanceof is reliable as-is. No change needed.
2. provider.tsx:156 (subscribes to both wallet events even when only one override is missing) — perf-only, no functional or UX effect. Deferred as nice-to-have.
3. provider.tsx:227 (raw provider error can propagate when wallet_switchEthereumChain fails for a non-rejection reason and no switchChainOverride exists) — this pointed at a real gap: CitizenClaimWidget's handlePrimaryAction catch only called onClaimError for primaryAction === 'claim', so a failed switch_chain (this case included) produced no toast and no callback at all — silent failure. Fixed in 17faecf: switch-chain failures now surface an error toast + onClaimError, naming the specific chain that was targeted, without leaking the raw wallet/provider error text.
4. WalletChip.tsx:50 (opacity-only disabled styling, potential contrast concern) — standard dimmed-disabled-control pattern, not a functional break and outside this PR's original scope. Deferred.

Re-requesting Copilot review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:111

• User-facing text has a grammatical error: change "wallets session" to "wallet's session".

                if (!onDisconnect) {
                  setDisconnectMessage('Disconnect should be done in your wallets session')
                  return
                }

packages/citizen-claim-widget/src/adapter.ts:67

• There are now two chain-name sources (getChainDisplayName here and getChainName in CitizenClaimWidget.tsx). This can lead to inconsistent chain naming between UI copy and adapter error messages as supported chains evolve. Consider centralizing chain display naming in a single shared utility (e.g., exporting one function from a common module used by both files) so all user-facing chain names remain consistent.

/** Resolves a supported chain id to its display name, falling back to the raw id. */
function getChainDisplayName(chainId: number): string {
  return CHAIN_CONFIGS[chainId]?.name ?? `Chain ${chainId}`
}

[goodbounties-nanoclaw-agent]

Round 11 Copilot review — 1 real comment, 2 suppressed.

• adapter.ts:696 — correctly caught that handleSwitchChain swallowed its error instead of rethrowing, so the switch-chain toast/callback added in 17faecf never actually ran. Fixed in 0ed69d5 (rethrows after setting status/error, matching handleClaim's existing pattern). Replied on the thread.
• WalletChip.tsx:111 (suppressed) — grammar typo ("wallets session" → "wallet's session"). Trivial, user-facing text fix — included in the same commit.
• adapter.ts:67 (suppressed) — suggests centralizing getChainDisplayName/getChainName into one shared utility to avoid future drift. Valid cleanup idea but no current inconsistency and no UX impact — deferred as nice-to-have.

Re-requesting Copilot review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

packages/citizen-claim-widget/src/adapter.ts:561

• This treats the custodial misconfiguration case (no configured clientsByChain found, so statusChainId === null) as unsupported_chain, which will drive the UI toward a network-switch narrative even though custodial mode doesn't depend on the wallet network. For isCustodialExecution && statusChainId === null, set status: 'error' (and a clear user-facing error) instead of unsupported_chain, reserving unsupported_chain for the non-custodial / active-chain-not-supported scenario.

    const statusChainId = isCustodialExecution
      ? SUPPORTED_CHAINS.find((supportedChainId) => claimExecution?.clientsByChain[supportedChainId]) ??
        null
      : chainId

    if (statusChainId === null || !isSupportedChain(statusChainId)) {
      await auxiliaryReads
      // Chain is known but unsupported (or custodial has no chain configured
      // at all) — a distinct status from not_connected so the UI can show
      // "switch chain" copy instead of misleadingly asking an already-connected
      // wallet to connect. Clear personalized entitlement from whatever chain
      // was previously active.
      setAmount(null)
      setNextClaimTime(null)
      setStatus('unsupported_chain')
      return
    }

packages/citizen-claim-widget/src/CitizenClaimWidget.tsx:624

• This introduces a new hard-coded chain id (42220) for the fallback network pill. Since the codebase already uses SupportedChains.CELO elsewhere (e.g. in switch-chain logic), prefer the shared constant here to avoid drift if the preferred/default chain changes.

        chainId={chainId ?? fallbackChainId ?? 42220}

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:66

• The interactive chip is an XStack with onPress, aria-label, and aria-disabled, but it likely renders as a non-interactive element (e.g. a div) without keyboard semantics. To make this accessible, render it as a semantic button component (preferred), or add role=\"button\", tabIndex={disabled ? -1 : 0}, and keyboard handlers (Enter/Space) consistent with the disabled state.

    <XStack position="relative" alignItems="center" opacity={disabled ? 0.5 : 1}>
      <XStack
        gap="$2"
        alignItems="center"
        paddingHorizontal="$3"
        paddingVertical="$2"
        borderRadius="$full"
        borderWidth={1}
        borderColor="$borderColor"
        cursor={disabled ? 'not-allowed' : 'pointer'}
        onPress={() => {
          if (disabled) return
          setDisconnectMessage(null)
          setIsMenuOpen((open) => !open)
        }}
        aria-label="Wallet options"
        aria-disabled={disabled}
      >

[goodbounties-nanoclaw-agent]

Round 12 Copilot review — 1 real comment, 3 suppressed.

• provider.tsx:230 — correctly flagged that wallet_switchEthereumChain had no timeout, so a wallet that never settles it (some WalletConnect sessions) could hang switchChain forever and never reach the override fallback. Fixed in e77f788: raced against a 10s timeout, treated like any other rejection. Replied on the thread.
• adapter.ts:561 (suppressed) — valid: custodial mode with no configured client for any supported chain was reported as unsupported_chain, driving a "switch network" narrative custodial users have no way to act on (no wallet chain to switch). Now reports error with a clear message instead. Fixed in e77f788.
• CitizenClaimWidget.tsx:624 (suppressed) — valid: replaced a hard-coded 42220 fallback with the existing SupportedChains.CELO constant. Fixed in e77f788.
• WalletChip.tsx:66 (suppressed) — keyboard/semantic-button accessibility gap on the chip's press target. Real, but pre-existing markup from an earlier PR (Add Superfluid Campaign GoodWidget #130), not introduced here, and warrants a proper pass across the component rather than a quick patch — deferred as out of scope for this PR.

Re-requesting Copilot review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:64

• The clickable chip is rendered as an XStack with onPress + aria-*, but without explicit button semantics (e.g. role="button" / keyboard activation). This can make the menu inaccessible to keyboard and some assistive-tech users. Prefer rendering this as an actual Button/pressable component from the UI kit, or add role="button", tabIndex={0}, and an onKeyDown handler for Enter/Space (and ensure aria-disabled is on an element with appropriate role).

    <XStack position="relative" alignItems="center" opacity={disabled ? 0.5 : 1}>
      <XStack
        gap="$2"
        alignItems="center"
        paddingHorizontal="$3"
        paddingVertical="$2"
        borderRadius="$full"
        borderWidth={1}
        borderColor="$borderColor"
        cursor={disabled ? 'not-allowed' : 'pointer'}
        onPress={() => {
          if (disabled) return
          setDisconnectMessage(null)
          setIsMenuOpen((open) => !open)
        }}
        aria-label="Wallet options"
        aria-disabled={disabled}
      >

packages/citizen-claim-widget/src/adapter.ts:409

• This allocates new publicClient/walletClient instances on every refresh for every supported chain, which can become expensive as loadClaimStatus/loadClaimablesByChain rerun (and can also increase connection churn). Consider caching clients per targetChainId (and per address where relevant) via a useMemo-managed Map or a ref-backed cache to reuse transports/clients across refreshes.

  const createReadOnlyClientsForChain = useCallback(
    (targetChainId: number) => {
      if (!address) return null

      if (isCustodialExecution) {
        const configuredClients = claimExecution?.clientsByChain[targetChainId]
        return configuredClients ? normalizeClientBundle(configuredClients) : null
      }

      const chain = CHAIN_CONFIGS[targetChainId]
      const rpcUrl = chain?.rpcUrls.default.http[0]
      if (!chain || !rpcUrl) return null

      const publicClient = createPublicClient({ chain, transport: http(rpcUrl) })
      const walletClient = createWalletClient({
        account: address as `0x${string}`,
        chain,
        transport: http(rpcUrl),
      })
      return { publicClient, walletClient }
    },
    [address, claimExecution, isCustodialExecution, normalizeClientBundle],
  )

[goodbounties-nanoclaw-agent]

Round 13 Copilot review — 1 real comment, 2 suppressed.

• App.tsx:88 — real: switchChainOverride resolved successfully even when it only opened AppKit's Networks modal (no direct descriptor, or switchNetwork failed), so callers proceeded as if the chain had actually switched. Fixed in b512ce0: throws instead, so the existing catch/toast handling (from earlier rounds) takes over rather than silently continuing on a switch that may not have happened.
• adapter.ts:409 (suppressed) — considered, not fixing: flags reallocating publicClient/walletClient on every refresh per chain. Both use viem's HTTP transport, which is stateless (fetch-based, no persistent connection to churn) — this is a minor object-allocation optimization, not a functional issue, and doesn't affect end-user UX. Deferred per scope.
• WalletChip.tsx:64 (suppressed) — same keyboard-accessibility gap flagged in round 12; still pre-existing markup from PR Add Superfluid Campaign GoodWidget #130, still deferred for the same reason (needs a broader a11y pass, not a quick patch here).

Re-requesting Copilot review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

apps/superfluid-campaign-web/src/App.tsx:24

• This error text is thrown as a plain Error, but CitizenClaimWidget’s humanReadableError() generally remaps unknown Error.message values to a generic fallback, so users may never see this specific guidance. To ensure this message is surfaced, either (a) update humanReadableError() to explicitly pass through this message (or a stable error code/prefix), or (b) throw an error whose message matches an existing “pass-through” path used by the widget (e.g., a recognizable substring that humanReadableError() maps deterministically).

const SWITCH_CHAIN_MANUAL_SELECTION_ERROR = 'Select the network in the wallet dialog, then try again.'

apps/superfluid-campaign-web/src/App.tsx:95

• This error text is thrown as a plain Error, but CitizenClaimWidget’s humanReadableError() generally remaps unknown Error.message values to a generic fallback, so users may never see this specific guidance. To ensure this message is surfaced, either (a) update humanReadableError() to explicitly pass through this message (or a stable error code/prefix), or (b) throw an error whose message matches an existing “pass-through” path used by the widget (e.g., a recognizable substring that humanReadableError() maps deterministically).

        if (!targetNetwork) {
          await open({ view: 'Networks' })
          throw new Error(SWITCH_CHAIN_MANUAL_SELECTION_ERROR)
        }
        try {
          await switchNetwork(targetNetwork)
        } catch {
          await open({ view: 'Networks' })
          throw new Error(SWITCH_CHAIN_MANUAL_SELECTION_ERROR)
        }

packages/citizen-claim-widget/src/adapter.ts:409

• This recreates publicClient/walletClient instances on each call (and loadClaimablesByChain calls this across all supported chains). Consider memoizing clients per (address, chainId) (e.g., via a ref-backed cache or useMemo keyed by address + supported chains) to avoid repeatedly constructing transports/clients during refreshes.

  const createReadOnlyClientsForChain = useCallback(
    (targetChainId: number) => {
      if (!address) return null

      if (isCustodialExecution) {
        const configuredClients = claimExecution?.clientsByChain[targetChainId]
        return configuredClients ? normalizeClientBundle(configuredClients) : null
      }

      const chain = CHAIN_CONFIGS[targetChainId]
      const rpcUrl = chain?.rpcUrls.default.http[0]
      if (!chain || !rpcUrl) return null

      const publicClient = createPublicClient({ chain, transport: http(rpcUrl) })
      const walletClient = createWalletClient({
        account: address as `0x${string}`,
        chain,
        transport: http(rpcUrl),
      })
      return { publicClient, walletClient }
    },
    [address, claimExecution, isCustodialExecution, normalizeClientBundle],
  )

[goodbounties-nanoclaw-agent]

Round 14 Copilot review — 1 real comment, 3 suppressed.

• adapter.ts:827 — real, and a meaningful one: for non-custodial execution, primaryAction returned 'claim' whenever any supported chain had a claimable balance, even when the active wallet chain was unsupported — but claimOnChain always executes against the active chain, so the button would offer "Claim" and immediately fail instead of prompting a chain switch first. Fixed in a001272: unsupported_chain now takes priority for non-custodial wallets; custodial (no single active chain) keeps its existing cross-chain precedence. Replied on the thread.
• App.tsx:24 / App.tsx:95 (suppressed, same issue twice) — valid: the new SWITCH_CHAIN_MANUAL_SELECTION_ERROR is a plain Error, so humanReadableError() in adapter.ts doesn't recognize it and falls back to a generic message in the inline error banner. The toast/callback path (CitizenClaimWidget's own chain-naming message) still fires correctly either way, so this only affects the secondary inline banner text. Not fixing as a string-match workaround — the correct fix is exporting the widget's pass-through CitizenClaimAdapterError as public API so integrators can throw it directly, which is a public-API decision I'd rather flag than make unilaterally. Let me know if you want that added.
• adapter.ts:409 (suppressed) — same client-recreation nit as round 13, still a non-issue (stateless HTTP transport, no connection to churn). Not fixing.

Re-requesting Copilot review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

packages/citizen-claim-widget/src/adapter.ts:409

• This creates new viem clients (and underlying transports) on every call per chain. Since loadClaimablesByChain fans out across SUPPORTED_CHAINS and may rerun frequently (refreshes, state transitions), this can add avoidable overhead. Consider caching publicClient/walletClient per targetChainId (e.g., a useMemo/useRef Map keyed by chainId + rpcUrl) and reusing them between calls, only invalidating when address (for walletClient.account) or RPC URL changes.

  const createReadOnlyClientsForChain = useCallback(
    (targetChainId: number) => {
      if (!address) return null

      if (isCustodialExecution) {
        const configuredClients = claimExecution?.clientsByChain[targetChainId]
        return configuredClients ? normalizeClientBundle(configuredClients) : null
      }

      const chain = CHAIN_CONFIGS[targetChainId]
      const rpcUrl = chain?.rpcUrls.default.http[0]
      if (!chain || !rpcUrl) return null

      const publicClient = createPublicClient({ chain, transport: http(rpcUrl) })
      const walletClient = createWalletClient({
        account: address as `0x${string}`,
        chain,
        transport: http(rpcUrl),
      })
      return { publicClient, walletClient }
    },
    [address, claimExecution, isCustodialExecution, normalizeClientBundle],
  )

packages/superfluid-campaign-widget/src/components/shared/WalletChip.tsx:67

• The new disabled state visually dims the chip and blocks onPress, but it still presents as an interactive control without a semantic disabled element. If XStack renders to a non-button element, aria-disabled alone may not fully convey/implement disabled behavior for assistive tech. Prefer using a semantic button/pressable component that supports a real disabled prop (or ensure the underlying element has appropriate role/keyboard handling and is removed from the tab order when disabled), so disabled behavior is consistent across mouse/keyboard/screen readers.

    <XStack position="relative" alignItems="center" opacity={disabled ? 0.5 : 1}>
      <XStack
        gap="$2"
        alignItems="center"
        paddingHorizontal="$3"
        paddingVertical="$2"
        borderRadius="$full"
        borderWidth={1}
        borderColor="$borderColor"
        cursor={disabled ? 'not-allowed' : 'pointer'}
        onPress={() => {
          if (disabled) return
          setDisconnectMessage(null)
          setIsMenuOpen((open) => !open)
        }}
        aria-label="Wallet options"
        aria-disabled={disabled}
      >

[goodbounties-nanoclaw-agent]

Round 15 Copilot review — 1 real comment (false positive), 2 suppressed (repeats).

• App.tsx:51 — checked, not a bug: flagged that Number(network.id) could produce NaN if AppKit network ids are CAIP-formatted strings. Verified DEFAULT_APPKIT_NETWORKS are all sourced from viem/chains (re-exported by @reown/appkit/networks), where EVM chain id is always a plain number — CAIP ids live on a separate caipNetworkId field this code never reads. No change made. Replied on the thread with the source reference.
• adapter.ts:409 (suppressed) — same client-recreation nit as rounds 13/14, still a non-issue (stateless HTTP transport).
• WalletChip.tsx:67 (suppressed) — same root cause as the round-12 keyboard-a11y nit (non-semantic interactive element), this time flagged via the disabled state specifically. Same reasoning applies — pre-existing markup, needs a dedicated a11y pass, out of scope here.

Re-requesting Copilot review.