Skip to content

feat(bounty): add FCFS single-claim flow with claim/approve UI and cl…#161

Open
wheval wants to merge 6 commits intoboundlessfi:mainfrom
wheval:feat/fcfs
Open

feat(bounty): add FCFS single-claim flow with claim/approve UI and cl…#161
wheval wants to merge 6 commits intoboundlessfi:mainfrom
wheval:feat/fcfs

Conversation

@wheval
Copy link
Copy Markdown
Contributor

@wheval wheval commented Mar 27, 2026

…aimed-state handling
Closes #140

Summary by CodeRabbit

  • New Features

    • Added claim functionality for fixed-price bounties with auto-release countdowns and abandonment warnings
    • Added bounty approval workflow for fixed-price bounty creators to review and release payments
    • Bounties now display a "Claimed" status when claimed in fixed-price mode
    • Added "In Review" status support for bounties
  • Chores

    • Internal notification tracking refactoring

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 27, 2026

@wheval is attempting to deploy a commit to the Threadflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 05500c00-9f09-4b38-9cd2-22e70c48b72b

📥 Commits

Reviewing files that changed from the base of the PR and between 6f23d07 and 653872d.

📒 Files selected for processing (1)
  • hooks/use-notifications.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • hooks/use-notifications.ts

📝 Walkthrough

Walkthrough

This PR implements the FCFS (First-Come-First-Served) bounty claiming flow. It adds components and hooks for contributors to claim FCFS bounties, shows "Claimed" status states, enables bounty creators to approve completed work, and includes auto-release countdowns for anti-squatting protection. Optimistic cache updates and error handling are implemented throughout the mutation flows.

Changes

Cohort / File(s) Summary
Status Badge Updates
components/bounty-detail/bounty-badges.tsx, components/bounty-detail/bounty-detail-header-card.tsx, components/bounty/bounty-card.tsx
Updated StatusBadge to accept optional type prop and conditionally render "Claimed" text for FCFS bounties in IN_PROGRESS state. bounty-card.tsx also normalizes status strings and adds in_review entry to statusConfig.
FCFS UI Components
components/bounty/fcfs-claim-button.tsx, components/bounty/fcfs-approval-panel.tsx
New components for FCFS claim flow: FcfsClaimButton handles claim/unclaim/countdown UI for contributors, while FcfsApprovalPanel enables creators to approve completed work and release payment with points validation.
Bounty Detail Integration
components/bounty-detail/bounty-detail-client.tsx, components/bounty-detail/bounty-detail-sidebar-cta.tsx
Conditionally render FcfsApprovalPanel for FIXED_PRICE bounties instead of submissions card. Updated sidebar CTA to show FcfsClaimButton for FCFS bounties and pass type prop to StatusBadge. Adjusted textarea minimum heights.
FCFS Mutation Hooks
hooks/use-claim-bounty.ts
New hook module exporting three mutation hooks (useClaimBounty, useApproveFcfs, useUnclaimBounty) with contract client resolution, credit validation, optimistic cache updates, and structured error handling via FcfsError.
Notification Hook Refactor
hooks/use-notifications.ts
Replaced prevUserIdRef with prevHydratedUserIdRef for user-change detection logic during render, preserving existing control flow with only ref variable rename.

Sequence Diagrams

sequenceDiagram
    participant User
    participant FcfsClaimButton
    participant useClaimBounty
    participant ContractClient
    participant QueryCache
    participant Toast

    User->>FcfsClaimButton: Click "Claim Bounty"
    FcfsClaimButton->>useClaimBounty: mutateAsync(bountyId, walletAddress)
    
    useClaimBounty->>ContractClient: Resolve FCFS client
    useClaimBounty->>ContractClient: Fetch contributor credits
    
    alt Insufficient Credits
        ContractClient-->>useClaimBounty: Credits < 1
        useClaimBounty-->>Toast: Show error
    else Credits Valid
        useClaimBounty->>QueryCache: Cancel & store bounty detail/list queries
        useClaimBounty->>ContractClient: Call claimBounty(bountyId)
        
        alt Claim Success
            ContractClient-->>useClaimBounty: Success
            useClaimBounty->>QueryCache: Optimistically update detail status to IN_PROGRESS
            useClaimBounty->>QueryCache: Update list cache entry status
            useClaimBounty-->>Toast: Show success
        else Claim Failed
            ContractClient-->>useClaimBounty: Error
            useClaimBounty->>QueryCache: Restore previous detail/list cache
            useClaimBounty-->>Toast: Show error
        end
        
        useClaimBounty->>QueryCache: Invalidate bounty detail & list queries
        useClaimBounty-->>FcfsClaimButton: Mutation complete
        FcfsClaimButton-->>User: UI updated / Already claimed state shown
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Benjtalkshow
  • 0xdevcollins

Poem

🐰 Hop-hop, the bounties now claim with a tap,
FCFS flows fill the contributor map,
Countdowns tick, wallets sign with delight,
Approval and payment, everything right! ✨🏆

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title correctly summarizes the main feature: adding FCFS single-claim flow with claim/approve UI and claimed-state handling.
Linked Issues check ✅ Passed All core requirements from issue #140 are implemented: claim button for FCFS bounties with credit check, optimistic updates, already-claimed state display, approval flow with payment release, auto-release countdowns, unclaim override, and cache invalidation.
Out of Scope Changes check ✅ Passed The only tangential change is a ref variable rename in use-notifications.ts (prevUserIdRef to prevHydratedUserIdRef), which is unrelated to FCFS feature but maintains existing behavior without functional impact.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@drips-wave
Copy link
Copy Markdown

drips-wave bot commented Mar 27, 2026

@wheval Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/bounty-detail/bounty-detail-sidebar-cta.tsx (1)

100-105: ⚠️ Potential issue | 🟡 Minor

Warning message shows incorrectly for FCFS bounties in claimed state.

When an FCFS bounty is IN_PROGRESS (claimed), canAct is false, so this warning displays alongside whatever FcfsClaimButton shows. The message "no longer accepting new submissions" is semantically incorrect for FCFS bounties—they're claimed, not closed to submissions. This warning should be scoped to non-FCFS bounties.

🐛 Proposed fix
-        {!canAct && (
+        {!canAct && !isFcfs && (
           <p className="flex items-center gap-1.5 text-xs text-gray-500 justify-center text-center">
             <AlertCircle className="size-3 shrink-0" />
             This bounty is no longer accepting new submissions.
           </p>
         )}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 100 -
105, The warning paragraph currently renders whenever canAct is false which
incorrectly shows for FCFS bounties that are claimed; update the rendering
condition to only show this message for non-FCFS bounties (e.g., change {!canAct
&& ( ... )} to something like {!canAct && bounty.fulfillmentType !== 'FCFS' && (
... )} or use the existing isFcfs/isFirstComeFirstServed flag if present) so the
AlertCircle message is scoped to non-FCFS bounties and does not appear alongside
FcfsClaimButton.
🧹 Nitpick comments (1)
components/bounty-detail/bounty-detail-sidebar-cta.tsx (1)

140-177: LGTM with optional refactor suggestion.

The FCFS conditional rendering is correctly implemented for mobile. The label() function is nearly identical to ctaLabel() in SidebarCTA (minus the CANCELLED case). Consider extracting a shared helper or a unified CTA component to reduce duplication, though this is not blocking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 140 -
177, The label logic in MobileCTA (function label()) duplicates the CTA label
logic in SidebarCTA (ctaLabel())—extract a shared helper (e.g., getCtaLabel or
useCtaLabel) and replace both label() in MobileCTA and ctaLabel() in SidebarCTA
to call that helper; keep the same switch cases (add CANCELLED if SidebarCTA
uses it) and ensure MobileCTA still uses FcfsClaimButton for FIXED_PRICE while
calling the shared helper for the button text and disabled logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/bounty-detail/bounty-detail-client.tsx`:
- Around line 74-75: BountyDetailSubmissionsCard is exposing the legacy “Submit
PR” and review/payment controls which breaks FCFS flow when FcfsApprovalPanel is
used; update components/bounty-detail/bounty-detail-submissions-card.tsx to
branch on the hideLegacyFcfsActions flag and bounty fields: if
hideLegacyFcfsActions is true and bounty.type is FCFS (or the FCFS-identifying
field), only render the submission UI when bounty.status === IN_PROGRESS and the
current user is the current claimant, and do NOT render the legacy
review/mark-paid controls for creators; otherwise keep existing behavior for
non-FCFS or when hideLegacyFcfsActions is false so the legacy paths remain
unchanged.

In `@components/bounty/bounty-card.tsx`:
- Around line 78-80: The isFcfsClaimed check compares bounty.status to
"IN_PROGRESS" while elsewhere bounty.status is used as a lower-case key into
statusConfig, so this comparison never matches; update the isFcfsClaimed logic
(the isFcfsClaimed constant) to normalize the status the same way the rest of
the file does (e.g., compare against the lower-case key used by statusConfig) or
convert bounty.status to upper-case consistently before comparing, keeping the
bounty.type check for "FIXED_PRICE" intact.

In `@components/bounty/fcfs-approval-panel.tsx`:
- Around line 30-36: The code currently falls back to currentUserId when
determining walletAddress, which can pass an application user id into on-chain
calls like approveFcfs; remove that fallback and ensure walletAddress is only
set from the session wallet fields (walletAddress or address) so it is undefined
when the user hasn’t connected a wallet, and update any callers (e.g.,
approveFcfs) or the component flow to bail out / show the “Connect your wallet”
UI when walletAddress is missing; specifically change how walletAddress is
derived (stop using currentUserId) and add a guard before calling approveFcfs
that requires a real walletAddress.

In `@components/bounty/fcfs-claim-button.tsx`:
- Around line 183-187: The pre-expiry warning is shown even for expired timers
because the predicate in fcfs-claim-button.tsx only checks if milestoneMsLeft or
responseMsLeft are less than 24h; update the condition used where isOwner is
checked (referencing isOwner, milestoneMsLeft, responseMsLeft) to also ensure
the remaining ms is greater than 0 (e.g., milestoneMsLeft != null &&
milestoneMsLeft > 0 && milestoneMsLeft < 24*60*60*1000, and likewise for
responseMsLeft) or alternatively branch earlier to render an explicit expired
state when either value is <= 0 so expired and “close to auto-release” states
don’t both render.
- Around line 75-81: The code currently falls back to session.user.id for
walletAddress (variables currentUserId and walletAddress), which can let
mutations run with an app user id instead of the signer address expected by the
FCFS contract; remove the fallback to currentUserId so walletAddress is derived
only from session?.user.walletAddress or session?.user.address, and update the
FCFS action entry points (handleClaim and handleUnclaim) to validate that
walletAddress exists and is a real wallet address (reject/disable the action if
absent or equal to the app user id), returning an error or prompting the signer
to connect a wallet rather than proceeding with the mutation.

In `@hooks/use-claim-bounty.ts`:
- Around line 69-82: applyDetailOptimisticStatus currently only flips
bounty.status so the UI (e.g., FcfsClaimButton) still sees no owner; change
applyDetailOptimisticStatus to accept an optional claimant identifier (e.g.,
claimedBy string or claimant object) and set the optimistic owner fields on the
returned bounty (at minimum bounty.claimedBy and bounty.claimedAt = new
Date().toISOString(), and any other owner fields your model uses) so the claim
mutation can pass the current claimer into the function and the detail cache
will reflect ownership immediately.
- Around line 130-157: onMutate currently saves previousDetail but not the
cached bounty lists, so update onMutate to also capture the list cache (e.g.
const previousLists =
queryClient.getQueryData<BountiesQuery>(bountyKeys.lists())) and return it in
the context; then in onError, in addition to restoring detail via
queryClient.setQueryData(bountyKeys.detail, context.previousDetail), restore the
lists cache (use queryClient.setQueryData or setQueriesData with
bountyKeys.lists() and context.previousLists) so list entries rolled to
"IN_PROGRESS" are reverted when getCredits/resolveContractClient/claimBounty
fails.

---

Outside diff comments:
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 100-105: The warning paragraph currently renders whenever canAct
is false which incorrectly shows for FCFS bounties that are claimed; update the
rendering condition to only show this message for non-FCFS bounties (e.g.,
change {!canAct && ( ... )} to something like {!canAct && bounty.fulfillmentType
!== 'FCFS' && ( ... )} or use the existing isFcfs/isFirstComeFirstServed flag if
present) so the AlertCircle message is scoped to non-FCFS bounties and does not
appear alongside FcfsClaimButton.

---

Nitpick comments:
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 140-177: The label logic in MobileCTA (function label())
duplicates the CTA label logic in SidebarCTA (ctaLabel())—extract a shared
helper (e.g., getCtaLabel or useCtaLabel) and replace both label() in MobileCTA
and ctaLabel() in SidebarCTA to call that helper; keep the same switch cases
(add CANCELLED if SidebarCTA uses it) and ensure MobileCTA still uses
FcfsClaimButton for FIXED_PRICE while calling the shared helper for the button
text and disabled logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: efdc0ffb-bd08-4c91-bd06-f3f7740062a3

📥 Commits

Reviewing files that changed from the base of the PR and between 3aad250 and a04c0df.

📒 Files selected for processing (10)
  • components/bounty-detail/bounty-badges.tsx
  • components/bounty-detail/bounty-detail-client.tsx
  • components/bounty-detail/bounty-detail-header-card.tsx
  • components/bounty-detail/bounty-detail-sidebar-cta.tsx
  • components/bounty-detail/bounty-detail-submissions-card.tsx
  • components/bounty/bounty-card.tsx
  • components/bounty/fcfs-approval-panel.tsx
  • components/bounty/fcfs-claim-button.tsx
  • hooks/__tests__/use-submission-draft.test.ts
  • hooks/use-claim-bounty.ts

Comment on lines +74 to +75
<BountyDetailSubmissionsCard bounty={bounty} />
<FcfsApprovalPanel bounty={bounty} />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

The legacy submissions card still breaks the FCFS flow.

BountyDetailSubmissionsCard is still the only place that exposes “Submit PR”, and it only does so for OPEN bounties. That means fixed-price users can submit before claiming, but once a claim moves the bounty to IN_PROGRESS, the claimant loses the only submission UI entirely. Rendering the unchanged legacy card next to FcfsApprovalPanel also leaves the old review/mark-paid controls active for creators.

Suggested direction
-        <BountyDetailSubmissionsCard bounty={bounty} />
-        <FcfsApprovalPanel bounty={bounty} />
+        <BountyDetailSubmissionsCard
+          bounty={bounty}
+          hideLegacyFcfsActions={bounty.type === "FIXED_PRICE"}
+        />
+        {bounty.type === "FIXED_PRICE" && (
+          <FcfsApprovalPanel bounty={bounty} />
+        )}

components/bounty-detail/bounty-detail-submissions-card.tsx then needs to branch on hideLegacyFcfsActions so FCFS bounties only allow submission from the current claimant in the claimed state, and no longer expose the legacy review/payment path.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<BountyDetailSubmissionsCard bounty={bounty} />
<FcfsApprovalPanel bounty={bounty} />
<BountyDetailSubmissionsCard
bounty={bounty}
hideLegacyFcfsActions={bounty.type === "FIXED_PRICE"}
/>
{bounty.type === "FIXED_PRICE" && (
<FcfsApprovalPanel bounty={bounty} />
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty-detail/bounty-detail-client.tsx` around lines 74 - 75,
BountyDetailSubmissionsCard is exposing the legacy “Submit PR” and
review/payment controls which breaks FCFS flow when FcfsApprovalPanel is used;
update components/bounty-detail/bounty-detail-submissions-card.tsx to branch on
the hideLegacyFcfsActions flag and bounty fields: if hideLegacyFcfsActions is
true and bounty.type is FCFS (or the FCFS-identifying field), only render the
submission UI when bounty.status === IN_PROGRESS and the current user is the
current claimant, and do NOT render the legacy review/mark-paid controls for
creators; otherwise keep existing behavior for non-FCFS or when
hideLegacyFcfsActions is false so the legacy paths remain unchanged.

Comment on lines +75 to +81
const currentUserId = (session?.user as { id?: string } | undefined)?.id;
const walletAddress =
(session?.user as { walletAddress?: string; address?: string } | undefined)
?.walletAddress ||
(session?.user as { walletAddress?: string; address?: string } | undefined)
?.address ||
currentUserId;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't use session.user.id as the contributor address.

This bypasses the wallet check in handleClaim/handleUnclaim and lets the mutation run with an app user id instead of the signer address expected by the FCFS contract calls.

Suggested fix
   const currentUserId = (session?.user as { id?: string } | undefined)?.id;
   const walletAddress =
     (session?.user as { walletAddress?: string; address?: string } | undefined)
-      ?.walletAddress ||
+      ?.walletAddress ??
     (session?.user as { walletAddress?: string; address?: string } | undefined)
-      ?.address ||
-    currentUserId;
+      ?.address ??
+    null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const currentUserId = (session?.user as { id?: string } | undefined)?.id;
const walletAddress =
(session?.user as { walletAddress?: string; address?: string } | undefined)
?.walletAddress ||
(session?.user as { walletAddress?: string; address?: string } | undefined)
?.address ||
currentUserId;
const currentUserId = (session?.user as { id?: string } | undefined)?.id;
const walletAddress =
(session?.user as { walletAddress?: string; address?: string } | undefined)
?.walletAddress ??
(session?.user as { walletAddress?: string; address?: string } | undefined)
?.address ??
null;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty/fcfs-claim-button.tsx` around lines 75 - 81, The code
currently falls back to session.user.id for walletAddress (variables
currentUserId and walletAddress), which can let mutations run with an app user
id instead of the signer address expected by the FCFS contract; remove the
fallback to currentUserId so walletAddress is derived only from
session?.user.walletAddress or session?.user.address, and update the FCFS action
entry points (handleClaim and handleUnclaim) to validate that walletAddress
exists and is a real wallet address (reject/disable the action if absent or
equal to the app user id), returning an error or prompting the signer to connect
a wallet rather than proceeding with the mutation.

Comment on lines +69 to +82
function applyDetailOptimisticStatus(
previous: BountyQuery | undefined,
nextStatus: string,
) {
if (!previous?.bounty) return previous;
return {
...previous,
bounty: {
...previous.bounty,
status: nextStatus,
updatedAt: new Date().toISOString(),
},
};
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Populate the optimistic owner fields during claim.

The detail cache only flips status, but FcfsClaimButton determines ownership from bounty.claimedBy. Right after a successful claim, the claimer will still see the generic “Already Claimed” state until the refetch lands, so the optimistic UI is incomplete.

Suggested fix
 function applyDetailOptimisticStatus(
   previous: BountyQuery | undefined,
   nextStatus: string,
+  claimedBy?: string,
 ) {
   if (!previous?.bounty) return previous;
   return {
     ...previous,
     bounty: {
       ...previous.bounty,
       status: nextStatus,
+      claimedBy: claimedBy ?? previous.bounty.claimedBy,
       updatedAt: new Date().toISOString(),
     },
   };
 }
-    onMutate: async ({ bountyId }) => {
+    onMutate: async ({ bountyId, contributorAddress }) => {
       await queryClient.cancelQueries({
         queryKey: bountyKeys.detail(bountyId),
       });
       const previousDetail = queryClient.getQueryData<BountyQuery>(
         bountyKeys.detail(bountyId),
       );

       queryClient.setQueryData<BountyQuery>(
         bountyKeys.detail(bountyId),
-        applyDetailOptimisticStatus(previousDetail, "IN_PROGRESS"),
+        applyDetailOptimisticStatus(
+          previousDetail,
+          "IN_PROGRESS",
+          contributorAddress,
+        ),
       );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/use-claim-bounty.ts` around lines 69 - 82, applyDetailOptimisticStatus
currently only flips bounty.status so the UI (e.g., FcfsClaimButton) still sees
no owner; change applyDetailOptimisticStatus to accept an optional claimant
identifier (e.g., claimedBy string or claimant object) and set the optimistic
owner fields on the returned bounty (at minimum bounty.claimedBy and
bounty.claimedAt = new Date().toISOString(), and any other owner fields your
model uses) so the claim mutation can pass the current claimer into the function
and the detail cache will reflect ownership immediately.

Copy link
Copy Markdown
Contributor

@Benjtalkshow Benjtalkshow left a comment

Choose a reason for hiding this comment

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

Hi @wheval , nice work on the FCFS claim flow,

The critical blockers are that BigInt(bountyId) in the hook will throw a SyntaxError if bounty IDs are UUIDs or non-numeric strings (which they appear to be elsewhere in the codebase), and the contract client is resolved from globalThis.__fcfsContracts which has no initialization guarantee. If the #139 bindings aren't loaded first, every FCFS action silently fails.

Additionally, walletAddress falls back to currentUserId in both fcfs-claim-button.tsx and fcfs-approval-panel.tsx, which means on-chain contract calls could receive an app user ID instead of a real wallet address. Remove this fallback and show a "Connect wallet" prompt instead.

Please also address the following:

Normalize bounty.status before the "Claimed" badge check in bounty-card.tsx.

Roll back list caches (not just detail) in onError.

Populate claimedBy in the optimistic update.

Gate the expired timer warning to > 0.

Conditionally render the FcfsApprovalPanel only for FIXED_PRICE bounties in bounty-detail-client.tsx.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
components/bounty-detail/bounty-detail-client.tsx (1)

74-75: ⚠️ Potential issue | 🟠 Major

Gate the legacy submissions card for FCFS.

BountyDetailSubmissionsCard is still rendered unconditionally next to FcfsApprovalPanel. If the legacy card still owns the old submit/review controls, FCFS users can see conflicting actions and the claimant can lose the submit path once the claim moves the bounty out of OPEN.

Expected result: components/bounty-detail/bounty-detail-submissions-card.tsx should branch on FCFS and suppress the legacy submit/review/payment controls.

#!/bin/bash
set -euo pipefail

sed -n '1,260p' components/bounty-detail/bounty-detail-submissions-card.tsx
echo
rg -n 'hideLegacyFcfsActions|FIXED_PRICE|IN_PROGRESS|UNDER_REVIEW|IN_REVIEW|Submit PR|Mark Paid|Approve' \
  components/bounty-detail/bounty-detail-submissions-card.tsx
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty-detail/bounty-detail-client.tsx` around lines 74 - 75, The
legacy submissions card is rendered unconditionally next to FcfsApprovalPanel,
causing conflicting controls for FIXED_PRICE bounties; update the UI so FCFS
bounties don't show legacy submit/review/payment actions by either (A) changing
the parent render to only show <BountyDetailSubmissionsCard bounty={bounty} />
when bounty.type !== "FIXED_PRICE", or (B) adding a prop like
hideLegacyFcfsActions and pass hideLegacyFcfsActions={bounty.type ===
"FIXED_PRICE"} and then inside BountyDetailSubmissionsCard branch on that prop
to suppress the legacy controls (look for strings/handlers tied to "Submit PR",
"Approve", "Mark Paid", or checks for IN_PROGRESS/UNDER_REVIEW/IN_REVIEW) so
FCFS flows only show FcfsApprovalPanel and not the old submission/payment
buttons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/bounty/bounty-card.tsx`:
- Around line 81-82: The status lookup `const status =
statusConfig[normalizedStatus.toLowerCase()] ?? statusConfig.open;` incorrectly
falls back to Open for "IN_REVIEW"; ensure "IN_REVIEW" maps to the existing
`under_review` key by normalizing or remapping `normalizedStatus` (e.g., convert
"IN_REVIEW" to "under_review" or check for "in_review"/"inreview"/"in-review")
before indexing `statusConfig`, so
`statusConfig[normalizedStatus.toLowerCase()]` returns
`statusConfig.under_review` for review bounties instead of defaulting to
`statusConfig.open`.

In `@components/bounty/fcfs-approval-panel.tsx`:
- Around line 83-91: Validate targetSubmission.githubPullRequestUrl before
rendering: parse it with the URL constructor and only render the anchor when
protocol === "https:" and hostname equals "github.com" (and optionally
"www.github.com"); additionally verify the pathname matches the GitHub PR
pattern (like /:owner/:repo/pull/:number). Update the FCFS approval panel code
that renders targetSubmission.githubPullRequestUrl to perform this check and
render the <a> only on success (otherwise omit the link or render a
non-clickable, escaped plain string). Ensure you use the validated URL variable
for both href and visible text so untrusted input is never used directly in
href.

In `@components/bounty/fcfs-claim-button.tsx`:
- Around line 126-127: The Unclaim handler (handleUnclaim) currently returns
early when walletAddress is missing which leaves the Unclaim dialog/Confirm
button enabled but non-functional; change the behavior so the Confirm action is
disabled or shows the same "connect-wallet" warning used in the approval flow
instead of silently no-op. Locate handleUnclaim and the Unclaim dialog/Confirm
button render and: 1) conditionally disable the Confirm button when
walletAddress is falsy, or 2) call the shared connect-wallet warning function
used by the approval panel before returning (so users get feedback). Apply the
same fix pattern to the other handlers/dialogs referenced in the file (the
similar code block around lines 203-249) to avoid any other silent no-ops.

In `@hooks/use-claim-bounty.ts`:
- Around line 148-153: The mutation currently only cancels/invalidates the
single list key via queryClient.cancelQueries({ queryKey: bountyKeys.lists() }),
which leaves other bounty list caches (active, organization, project) stale;
replace those calls to instead iterate over or call
queryClient.cancelQueries/invalidateQueries for each key in
bountyKeys.allListKeys (or call cancel/invalidate for each entry returned by
bountyKeys.allListKeys()) so all bounty list families are refreshed after
claim/approve/unclaim; update every occurrence that currently uses
bountyKeys.lists() (e.g., in the handlers that call
queryClient.cancelQueries/invalidateQueries) to use bountyKeys.allListKeys.

---

Duplicate comments:
In `@components/bounty-detail/bounty-detail-client.tsx`:
- Around line 74-75: The legacy submissions card is rendered unconditionally
next to FcfsApprovalPanel, causing conflicting controls for FIXED_PRICE
bounties; update the UI so FCFS bounties don't show legacy submit/review/payment
actions by either (A) changing the parent render to only show
<BountyDetailSubmissionsCard bounty={bounty} /> when bounty.type !==
"FIXED_PRICE", or (B) adding a prop like hideLegacyFcfsActions and pass
hideLegacyFcfsActions={bounty.type === "FIXED_PRICE"} and then inside
BountyDetailSubmissionsCard branch on that prop to suppress the legacy controls
(look for strings/handlers tied to "Submit PR", "Approve", "Mark Paid", or
checks for IN_PROGRESS/UNDER_REVIEW/IN_REVIEW) so FCFS flows only show
FcfsApprovalPanel and not the old submission/payment buttons.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 737aef99-04c8-4ff3-957c-a83396952838

📥 Commits

Reviewing files that changed from the base of the PR and between a04c0df and 49e5a0d.

📒 Files selected for processing (5)
  • components/bounty-detail/bounty-detail-client.tsx
  • components/bounty/bounty-card.tsx
  • components/bounty/fcfs-approval-panel.tsx
  • components/bounty/fcfs-claim-button.tsx
  • hooks/use-claim-bounty.ts

Comment on lines +81 to +82
const status =
statusConfig[normalizedStatus.toLowerCase()] ?? statusConfig.open;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Map IN_REVIEW before defaulting to Open.

FcfsApprovalPanel in this PR treats "IN_REVIEW" as a valid review state, but statusConfig only defines under_review. When Line 82 resolves an IN_REVIEW bounty, it falls through to statusConfig.open, so review bounties render as Open on cards.

Suggested fix
 const statusConfig: Record<
   string,
   {
     variant: "default" | "secondary" | "outline" | "destructive";
     label: string;
     dotColor: string;
   }
 > = {
+  in_review: {
+    variant: "secondary",
+    label: "In Review",
+    dotColor: "bg-amber-500",
+  },
   open: {
     variant: "default",
     label: "Open",
     dotColor: "bg-emerald-500",
   },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const status =
statusConfig[normalizedStatus.toLowerCase()] ?? statusConfig.open;
const statusConfig: Record<
string,
{
variant: "default" | "secondary" | "outline" | "destructive";
label: string;
dotColor: string;
}
> = {
in_review: {
variant: "secondary",
label: "In Review",
dotColor: "bg-amber-500",
},
open: {
variant: "default",
label: "Open",
dotColor: "bg-emerald-500",
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty/bounty-card.tsx` around lines 81 - 82, The status lookup
`const status = statusConfig[normalizedStatus.toLowerCase()] ??
statusConfig.open;` incorrectly falls back to Open for "IN_REVIEW"; ensure
"IN_REVIEW" maps to the existing `under_review` key by normalizing or remapping
`normalizedStatus` (e.g., convert "IN_REVIEW" to "under_review" or check for
"in_review"/"inreview"/"in-review") before indexing `statusConfig`, so
`statusConfig[normalizedStatus.toLowerCase()]` returns
`statusConfig.under_review` for review bounties instead of defaulting to
`statusConfig.open`.

Comment on lines +83 to +91
{targetSubmission.githubPullRequestUrl && (
<a
href={targetSubmission.githubPullRequestUrl}
target="_blank"
rel="noopener noreferrer"
className="text-primary hover:underline break-all"
>
{targetSubmission.githubPullRequestUrl}
</a>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate the PR URL before rendering it as a link.

githubPullRequestUrl is submission metadata, so it is user-controlled. Rendering it directly into href lets javascript:/data: URLs execute when the creator clicks the link. Parse it first and allow only expected https GitHub URLs.

Suggested fix
   const targetSubmission = bounty.submissions?.[0];
+  const safePullRequestUrl = (() => {
+    const raw = targetSubmission?.githubPullRequestUrl;
+    if (!raw) return null;
+    try {
+      const url = new URL(raw);
+      if (
+        url.protocol !== "https:" ||
+        !/^(www\.)?github\.com$/i.test(url.hostname)
+      ) {
+        return null;
+      }
+      return url.toString();
+    } catch {
+      return null;
+    }
+  })();

   return (
@@
-          {targetSubmission.githubPullRequestUrl && (
+          {safePullRequestUrl && (
             <a
-              href={targetSubmission.githubPullRequestUrl}
+              href={safePullRequestUrl}
               target="_blank"
               rel="noopener noreferrer"
               className="text-primary hover:underline break-all"
             >
-              {targetSubmission.githubPullRequestUrl}
+              {safePullRequestUrl}
             </a>
           )}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty/fcfs-approval-panel.tsx` around lines 83 - 91, Validate
targetSubmission.githubPullRequestUrl before rendering: parse it with the URL
constructor and only render the anchor when protocol === "https:" and hostname
equals "github.com" (and optionally "www.github.com"); additionally verify the
pathname matches the GitHub PR pattern (like /:owner/:repo/pull/:number). Update
the FCFS approval panel code that renders targetSubmission.githubPullRequestUrl
to perform this check and render the <a> only on success (otherwise omit the
link or render a non-clickable, escaped plain string). Ensure you use the
validated URL variable for both href and visible text so untrusted input is
never used directly in href.

Comment on lines +126 to +127
const handleUnclaim = async () => {
if (!walletAddress) return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Don't let unclaim submit into a silent no-op.

Line 127 returns immediately when no wallet is connected, but the Unclaim dialog and confirm button stay enabled for creators. That makes the override look available while clicking it does nothing. Disable the action or surface the same connect-wallet warning used in the approval panel.

Suggested fix
-  const handleUnclaim = async () => {
-    if (!walletAddress) return;
+  const handleUnclaim = async () => {
+    if (!walletAddress) {
+      toast.error("Connect your wallet to unclaim this bounty.");
+      return;
+    }
     try {
       await unclaimMutation.mutateAsync({
         bountyId: bounty.id,
@@
-          {isCreator && (
+          {isCreator && !walletAddress && (
+            <p className="text-sm text-amber-400 text-center py-2">
+              Connect your wallet to unclaim this bounty.
+            </p>
+          )}
+
+          {isCreator && (
             <Dialog open={unclaimOpen} onOpenChange={setUnclaimOpen}>
               <DialogTrigger asChild>
-                <Button variant="outline" size="sm" className="w-full">
+                <Button
+                  variant="outline"
+                  size="sm"
+                  className="w-full"
+                  disabled={!walletAddress}
+                >
                   Unclaim
                 </Button>
               </DialogTrigger>
@@
                   <Button
                     onClick={() => void handleUnclaim()}
                     disabled={
-                      !justification.trim() || unclaimMutation.isPending
+                      !walletAddress ||
+                      !justification.trim() ||
+                      unclaimMutation.isPending
                     }
                   >

Also applies to: 203-249

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty/fcfs-claim-button.tsx` around lines 126 - 127, The Unclaim
handler (handleUnclaim) currently returns early when walletAddress is missing
which leaves the Unclaim dialog/Confirm button enabled but non-functional;
change the behavior so the Confirm action is disabled or shows the same
"connect-wallet" warning used in the approval flow instead of silently no-op.
Locate handleUnclaim and the Unclaim dialog/Confirm button render and: 1)
conditionally disable the Confirm button when walletAddress is falsy, or 2) call
the shared connect-wallet warning function used by the approval panel before
returning (so users get feedback). Apply the same fix pattern to the other
handlers/dialogs referenced in the file (the similar code block around lines
203-249) to avoid any other silent no-ops.

Comment on lines +148 to +153
await queryClient.cancelQueries({
queryKey: bountyKeys.detail(bountyId),
});
await queryClient.cancelQueries({
queryKey: bountyKeys.lists(),
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Invalidate all bounty list families, not just ["Bounties"].

These mutations only cancel/update/invalidate bountyKeys.lists(), but lib/query/query-keys.ts already exposes bountyKeys.allListKeys for the active, organization, and project bounty caches too. After a claim/approve/unclaim, those views can keep showing stale status or claim state until a hard refetch.

Also applies to: 170-173, 191-194, 214-216, 245-248, 275-278

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/use-claim-bounty.ts` around lines 148 - 153, The mutation currently
only cancels/invalidates the single list key via queryClient.cancelQueries({
queryKey: bountyKeys.lists() }), which leaves other bounty list caches (active,
organization, project) stale; replace those calls to instead iterate over or
call queryClient.cancelQueries/invalidateQueries for each key in
bountyKeys.allListKeys (or call cancel/invalidate for each entry returned by
bountyKeys.allListKeys()) so all bounty list families are refreshed after
claim/approve/unclaim; update every occurrence that currently uses
bountyKeys.lists() (e.g., in the handlers that call
queryClient.cancelQueries/invalidateQueries) to use bountyKeys.allListKeys.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
components/bounty/bounty-card.tsx (1)

83-84: ⚠️ Potential issue | 🟠 Major

Remap IN_REVIEW before the Open fallback.

On Line 84, IN_REVIEW becomes in_review, but statusConfig only defines under_review, so review bounties can render as Open.

Suggested fix
-  const status =
-    statusConfig[normalizedStatus.toLowerCase()] ?? statusConfig.open;
+  const statusKey =
+    normalizedStatus === "IN_REVIEW"
+      ? "under_review"
+      : normalizedStatus.toLowerCase();
+  const status = statusConfig[statusKey] ?? statusConfig.open;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty/bounty-card.tsx` around lines 83 - 84, The status fallback
currently maps via normalizedStatus.toLowerCase() into statusConfig and falls
back to statusConfig.open, which causes "IN_REVIEW" (normalized to "in_review")
to miss the existing "under_review" key; before computing status, remap the
normalizedStatus value (handle the "IN_REVIEW" / "in_review" case) to the
matching key used by statusConfig (e.g., convert "in_review" to "under_review")
so that the lookup for statusConfig[normalizedStatus] returns the correct
under_review entry instead of falling back to statusConfig.open; update the
logic around normalizedStatus/status and reference the normalizedStatus variable
and statusConfig lookup to implement this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hooks/use-notifications.ts`:
- Around line 93-104: The current re-hydration logic uses prevHydratedUserIdRef
initialized to userId, so when the initial resolved session is unauthenticated
(userId === null) the branch never runs and hydrated remains false; change the
logic so the first resolution (including anonymous/null userId) sets hydrated
and loads storage: initialize prevHydratedUserIdRef to undefined (or add a
firstRenderRef) so that prevHydratedUserIdRef.current !== userId on first pass,
then call setNotifications(userId ? loadFromStorage(userId) : []) and
setHydrated(true); reference the useState hooks (notifications, hydrated),
prevHydratedUserIdRef, setNotifications, loadFromStorage and setHydrated to
locate and update the code.

---

Duplicate comments:
In `@components/bounty/bounty-card.tsx`:
- Around line 83-84: The status fallback currently maps via
normalizedStatus.toLowerCase() into statusConfig and falls back to
statusConfig.open, which causes "IN_REVIEW" (normalized to "in_review") to miss
the existing "under_review" key; before computing status, remap the
normalizedStatus value (handle the "IN_REVIEW" / "in_review" case) to the
matching key used by statusConfig (e.g., convert "in_review" to "under_review")
so that the lookup for statusConfig[normalizedStatus] returns the correct
under_review entry instead of falling back to statusConfig.open; update the
logic around normalizedStatus/status and reference the normalizedStatus variable
and statusConfig lookup to implement this change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 80674d7c-836c-4c3d-8d6d-0b87096e98fa

📥 Commits

Reviewing files that changed from the base of the PR and between 49e5a0d and 731d87b.

📒 Files selected for processing (4)
  • components/bounty-detail/bounty-detail-client.tsx
  • components/bounty/bounty-card.tsx
  • components/bounty/fee-calculator.tsx
  • hooks/use-notifications.ts
✅ Files skipped from review due to trivial changes (2)
  • components/bounty/fee-calculator.tsx
  • components/bounty-detail/bounty-detail-client.tsx

@wheval wheval requested a review from Benjtalkshow March 30, 2026 10:11
@wheval
Copy link
Copy Markdown
Contributor Author

wheval commented Mar 30, 2026

@Benjtalkshow please review

Copy link
Copy Markdown
Contributor

@Benjtalkshow Benjtalkshow left a comment

Choose a reason for hiding this comment

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

@wheval , great progress on the fixes. The BigInt crash, wallet address fallback, optimistic cache rollback, claimedBy field, expired timer guard, and status normalization are all properly addressed now, nice work!

There are a few remaining items:

githubPullRequestUrl from submission metadata is rendered directly as an <a href> in fcfs-approval-panel.tsx without sanitization. A javascript: URL could execute on click, so validate that it starts with https://. Also use next/link instead of native anchor tag

IN_REVIEW status is missing from statusConfig in bounty-card.tsx, so review bounties fall through to the "Open" default.

The legacy BountyDetailSubmissionsCard still renders alongside FcfsApprovalPanel for FCFS bounties, which exposes conflicting submit and review controls.

Please also check if hooks/use-notifications.ts and components/bounty/fee-calculator.tsx were accidentally included from a merge, as they appear out of scope for this FCFS PR.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
components/bounty/fcfs-approval-panel.tsx (1)

84-93: ⚠️ Potential issue | 🟠 Major

Harden PR link validation before rendering clickable URLs.

Line 84-Line 93 still treats any https:// URL as trusted. This allows attacker-controlled non-GitHub links to be shown as official submission links. Validate with URL, restrict host to GitHub, and enforce PR path format before rendering <Link>.

Suggested fix
   const targetSubmission = bounty.submissions?.[0];
+  const safePullRequestUrl = (() => {
+    const raw = targetSubmission?.githubPullRequestUrl;
+    if (!raw) return null;
+    try {
+      const url = new URL(raw);
+      const isGithubHost = /^(www\.)?github\.com$/i.test(url.hostname);
+      const isPrPath = /^\/[^/]+\/[^/]+\/pull\/\d+\/?$/.test(url.pathname);
+      if (url.protocol !== "https:" || !isGithubHost || !isPrPath) return null;
+      return url.toString();
+    } catch {
+      return null;
+    }
+  })();
@@
-          {targetSubmission.githubPullRequestUrl &&
-          targetSubmission.githubPullRequestUrl.startsWith("https://") ? (
+          {safePullRequestUrl ? (
             <Link
-              href={targetSubmission.githubPullRequestUrl}
+              href={safePullRequestUrl}
               target="_blank"
               rel="noopener noreferrer"
               className="text-primary hover:underline break-all"
             >
-              {targetSubmission.githubPullRequestUrl}
+              {safePullRequestUrl}
             </Link>
-          ) : targetSubmission.githubPullRequestUrl ? (
+          ) : targetSubmission.githubPullRequestUrl ? (
             <span className="text-xs text-gray-400 break-all">
               {targetSubmission.githubPullRequestUrl}
             </span>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty/fcfs-approval-panel.tsx` around lines 84 - 93, The PR link
rendering currently trusts any URL starting with "https://"; add robust
validation before rendering the Link by parsing
targetSubmission.githubPullRequestUrl with the URL constructor, ensuring
url.hostname === "github.com" (optionally allow "www.github.com"), and verifying
the pathname matches the GitHub PR pattern (e.g.
/^\/[^/]+\/[^/]+\/pull\/\d+($|\/)/) — implement a small helper like
isValidGitHubPRUrl(urlString) and only render the <Link> when that helper
returns true, otherwise render a safe fallback (plain text or no link).
🧹 Nitpick comments (1)
components/bounty-detail/bounty-detail-sidebar-cta.tsx (1)

282-294: Minor inconsistency: label() missing CANCELLED case.

ctaLabel() in SidebarCTA (line 74-75) handles the CANCELLED status explicitly, returning "Cancelled", while label() here falls through to "Not Available". Consider adding the missing case for consistency.

Proposed fix
       switch (bounty.status) {
         case "IN_PROGRESS":
           return "In Progress";
         case "COMPLETED":
           return "Completed";
+        case "CANCELLED":
+          return "Cancelled";
         default:
           return "Not Available";
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 282 -
294, The label() function in bounty-detail-sidebar-cta.tsx doesn't handle the
"CANCELLED" bounty.status and falls through to "Not Available", causing
inconsistency with SidebarCTA.ctaLabel() which returns "Cancelled"; update
label() to include a case "CANCELLED" that returns "Cancelled" so both functions
produce the same user-facing string for cancelled bounties (refer to label() and
ctaLabel()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 318-326: The icon-only cancel Button in
bounty-detail-sidebar-cta.tsx lacks an accessible name; update the Button (the
one with onClick={() => setCancelDialogOpen(true)} and containing <XCircle>) to
include an aria-label (e.g., aria-label="Cancel" or a localized string) so
screen readers can announce its purpose, and ensure the label is meaningful and
matches any existing localization patterns used elsewhere in the component.
- Around line 298-328: The FCFS branch currently only renders <FcfsClaimButton>
so creators don't see the cancel button; update the conditional rendering around
isFcfs in bounty-detail-sidebar-cta to always render the cancel-control
alongside FcfsClaimButton when canCancel is true — reuse the same cancel Button
logic that calls setCancelDialogOpen(true) and uses the XCircle icon (same
appearance/props as in the non-FCFS branch) so FCFS bounties show the cancel
button for creators just like SidebarCTA does.

In `@components/bounty/fcfs-approval-panel.tsx`:
- Around line 55-56: The current validation only checks Number.isFinite(points)
and points < 0, allowing fractional values; update validation to require integer
points by using Number.isInteger(points) && points >= 0 and change the input
parsing logic (where the raw numeric input is parsed into points—e.g., the
onChange/parse handler that currently uses Number(...) or parseFloat) to
parse/sanitize as an integer (use parseInt or Math.round/Math.floor as
appropriate for your UX) and ensure the toast.error message is shown when
!Number.isInteger(points) or points < 0; also enforce integer-only input at the
form control (e.g., number input with step=1 or reject values containing a
decimal) so both the UI and the validation in the FCFSApprovalPanel (the points
variable and its setter/parse handler) consistently block fractional
reputations.

---

Duplicate comments:
In `@components/bounty/fcfs-approval-panel.tsx`:
- Around line 84-93: The PR link rendering currently trusts any URL starting
with "https://"; add robust validation before rendering the Link by parsing
targetSubmission.githubPullRequestUrl with the URL constructor, ensuring
url.hostname === "github.com" (optionally allow "www.github.com"), and verifying
the pathname matches the GitHub PR pattern (e.g.
/^\/[^/]+\/[^/]+\/pull\/\d+($|\/)/) — implement a small helper like
isValidGitHubPRUrl(urlString) and only render the <Link> when that helper
returns true, otherwise render a safe fallback (plain text or no link).

---

Nitpick comments:
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 282-294: The label() function in bounty-detail-sidebar-cta.tsx
doesn't handle the "CANCELLED" bounty.status and falls through to "Not
Available", causing inconsistency with SidebarCTA.ctaLabel() which returns
"Cancelled"; update label() to include a case "CANCELLED" that returns
"Cancelled" so both functions produce the same user-facing string for cancelled
bounties (refer to label() and ctaLabel()).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b524cdbe-bfb6-476f-8ad9-d1c8abe8fd6f

📥 Commits

Reviewing files that changed from the base of the PR and between 731d87b and 6f23d07.

📒 Files selected for processing (7)
  • components/bounty-detail/bounty-detail-client.tsx
  • components/bounty-detail/bounty-detail-sidebar-cta.tsx
  • components/bounty/bounty-card.tsx
  • components/bounty/fcfs-approval-panel.tsx
  • hooks/use-notifications.ts
  • lib/contracts/index.ts
  • lib/contracts/transaction.ts
✅ Files skipped from review due to trivial changes (2)
  • lib/contracts/index.ts
  • lib/contracts/transaction.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • components/bounty/bounty-card.tsx
  • components/bounty-detail/bounty-detail-client.tsx

Comment on lines +298 to +328
{isFcfs ? (
<FcfsClaimButton bounty={bounty} />
) : (
<div className="flex gap-2">
<Button
variant="outline"
className="flex-1 h-11 font-bold tracking-wide"
disabled={!canAct}
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
onClick={() =>
canAct &&
window.open(
bounty.githubIssueUrl,
"_blank",
"noopener,noreferrer",
)
}
>
<XCircle className="size-4" />
{label()}
</Button>
)}
</div>
{canCancel && (
<Button
variant="outline"
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
>
<XCircle className="size-4" />
</Button>
)}
</div>
)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

FCFS bounties on mobile are missing the cancel button for creators.

When isFcfs is true, only <FcfsClaimButton> is rendered, but the cancel button (for canCancel cases) is placed inside the else branch. This means bounty creators cannot cancel FCFS bounties from mobile, unlike in SidebarCTA where the cancel button (lines 147-161) is rendered outside the FCFS conditional.

Proposed fix to include cancel button for FCFS bounties
     {isFcfs ? (
-      <FcfsClaimButton bounty={bounty} />
+      <div className="flex gap-2">
+        <div className="flex-1">
+          <FcfsClaimButton bounty={bounty} />
+        </div>
+        {canCancel && (
+          <Button
+            variant="outline"
+            size="lg"
+            className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
+            onClick={() => setCancelDialogOpen(true)}
+            aria-label="Cancel bounty"
+          >
+            <XCircle className="size-4" />
+          </Button>
+        )}
+      </div>
     ) : (
       <div className="flex gap-2">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{isFcfs ? (
<FcfsClaimButton bounty={bounty} />
) : (
<div className="flex gap-2">
<Button
variant="outline"
className="flex-1 h-11 font-bold tracking-wide"
disabled={!canAct}
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
onClick={() =>
canAct &&
window.open(
bounty.githubIssueUrl,
"_blank",
"noopener,noreferrer",
)
}
>
<XCircle className="size-4" />
{label()}
</Button>
)}
</div>
{canCancel && (
<Button
variant="outline"
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
>
<XCircle className="size-4" />
</Button>
)}
</div>
)}
{isFcfs ? (
<div className="flex gap-2">
<div className="flex-1">
<FcfsClaimButton bounty={bounty} />
</div>
{canCancel && (
<Button
variant="outline"
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
aria-label="Cancel bounty"
>
<XCircle className="size-4" />
</Button>
)}
</div>
) : (
<div className="flex gap-2">
<Button
className="flex-1 h-11 font-bold tracking-wide"
disabled={!canAct}
size="lg"
onClick={() =>
canAct &&
window.open(
bounty.githubIssueUrl,
"_blank",
"noopener,noreferrer",
)
}
>
{label()}
</Button>
{canCancel && (
<Button
variant="outline"
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
>
<XCircle className="size-4" />
</Button>
)}
</div>
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 298 -
328, The FCFS branch currently only renders <FcfsClaimButton> so creators don't
see the cancel button; update the conditional rendering around isFcfs in
bounty-detail-sidebar-cta to always render the cancel-control alongside
FcfsClaimButton when canCancel is true — reuse the same cancel Button logic that
calls setCancelDialogOpen(true) and uses the XCircle icon (same appearance/props
as in the non-FCFS branch) so FCFS bounties show the cancel button for creators
just like SidebarCTA does.

Comment on lines +318 to +326
<Button
variant="outline"
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
>
<XCircle className="size-4" />
</Button>
)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add aria-label to the icon-only cancel button.

The cancel button only contains an icon with no accessible name. Screen reader users cannot determine its purpose.

Proposed fix
           <Button
             variant="outline"
             size="lg"
             className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
             onClick={() => setCancelDialogOpen(true)}
+            aria-label="Cancel bounty"
           >
             <XCircle className="size-4" />
           </Button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
variant="outline"
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
>
<XCircle className="size-4" />
</Button>
)}
<Button
variant="outline"
size="lg"
className="h-11 border-red-500/30 text-red-400 hover:bg-red-500/10 shrink-0"
onClick={() => setCancelDialogOpen(true)}
aria-label="Cancel bounty"
>
<XCircle className="size-4" />
</Button>
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 318 -
326, The icon-only cancel Button in bounty-detail-sidebar-cta.tsx lacks an
accessible name; update the Button (the one with onClick={() =>
setCancelDialogOpen(true)} and containing <XCircle>) to include an aria-label
(e.g., aria-label="Cancel" or a localized string) so screen readers can announce
its purpose, and ensure the label is meaningful and matches any existing
localization patterns used elsewhere in the component.

Comment on lines +55 to +56
if (!Number.isFinite(points) || points < 0) {
toast.error("Points must be a valid non-negative number.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Enforce integer points to avoid unintended approvals.

Line 55-Line 56 allows decimal values because only finite/non-negative is checked, and Line 109-Line 115 parses raw numeric input directly. If points are discrete reputation units, fractional values should be blocked at input + validation layers.

Suggested fix
-    if (!Number.isFinite(points) || points < 0) {
+    if (!Number.isFinite(points) || points < 0 || !Number.isInteger(points)) {
       toast.error("Points must be a valid non-negative number.");
       return;
     }
@@
         <Input
           id="fcfs-approval-points"
           type="number"
           min={0}
+          step={1}
           value={points}
           onChange={(e) => setPoints(Number(e.target.value))}
         />

Also applies to: 109-115

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/bounty/fcfs-approval-panel.tsx` around lines 55 - 56, The current
validation only checks Number.isFinite(points) and points < 0, allowing
fractional values; update validation to require integer points by using
Number.isInteger(points) && points >= 0 and change the input parsing logic
(where the raw numeric input is parsed into points—e.g., the onChange/parse
handler that currently uses Number(...) or parseFloat) to parse/sanitize as an
integer (use parseInt or Math.round/Math.floor as appropriate for your UX) and
ensure the toast.error message is shown when !Number.isInteger(points) or points
< 0; also enforce integer-only input at the form control (e.g., number input
with step=1 or reject values containing a decimal) so both the UI and the
validation in the FCFSApprovalPanel (the points variable and its setter/parse
handler) consistently block fractional reputations.

@wheval wheval requested a review from Benjtalkshow April 6, 2026 16:17
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.

Implement FCFS (Single Claim) Bounty Flow

2 participants