feat: implement maintainer dashboard actions for multi-winner milestone bounties#265
Conversation
|
@Jopsan-gm is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds bounty-scoped mutation hooks and EscrowService.releasePayment, wires Model4MaintainerDashboard to use those hooks with per-contributor loading and two dialogs, and updates CI Node version and Playwright install step. ChangesModel 4 Maintainer Dashboard Actions Implementation
Sequence Diagram(s)sequenceDiagram
participant User as Maintainer (UI)
participant Dashboard as Model4MaintainerDashboard
participant Hook as useReleasePayment / useAdvanceContributor / useRemoveContributor / useSendMessage
participant QueryClient as ReactQueryCache
participant Escrow as EscrowService
User->>Dashboard: click action (Release/Advance/Remove/Message/View Submissions)
Dashboard->>Hook: mutate({ contributorId, ... }) or open dialog
Hook->>QueryClient: cancel/read/update optimistic cache (bountyDetail/escrow)
alt releasePayment
Hook->>Escrow: releasePayment(poolId, amount)
Escrow-->>Hook: updated EscrowPool
Hook->>QueryClient: update escrow cache + invalidate bountyDetail + escrow queries
else advance/remove
Hook->>QueryClient: optimistic update contributorProgress or remove contributor -> invalidate on success
else sendMessage
Hook->>Hook: store message in recordedMessages (no cache change)
end
Hook-->>Dashboard: mutation success/failure (toast, close dialog)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
hooks/use-bounty-application.ts (2)
32-88: ⚡ Quick winMissing
onSettledinvalidation for optimistic mutations.
advanceContributorandremoveContributoruse optimistic updates but never reconcile with the server after the mutation completes. If the server state differs from the optimistic update (e.g., another maintainer made changes), the cache will remain stale until the user manually refreshes.Add onSettled to invalidate and refetch
const advanceContributor = useMutation({ // ... existing code ... onError: (_err, _vars, context) => { if (context?.previous) { queryClient.setQueryData(bountyKeys.detail(bountyId), context.previous); } }, + onSettled: () => { + queryClient.invalidateQueries({ queryKey: bountyKeys.detail(bountyId) }); + }, }); const removeContributor = useMutation({ // ... existing code ... onError: (_err, _vars, context) => { if (context?.previous) { queryClient.setQueryData(bountyKeys.detail(bountyId), context.previous); } }, + onSettled: () => { + queryClient.invalidateQueries({ queryKey: bountyKeys.detail(bountyId) }); + }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/use-bounty-application.ts` around lines 32 - 88, The optimistic update in advanceContributor (and similarly in removeContributor) never reconciles with the server; add an onSettled handler to the useMutation config for advanceContributor that calls queryClient.invalidateQueries(bountyKeys.detail(bountyId)) (and optionally queryClient.refetchQueries) to ensure the bounty detail cache is refetched after success or error; locate the useMutation for advanceContributor (and the removeContributor mutation) and append an onSettled: (_data, _error, _vars, _context) => { queryClient.invalidateQueries(bountyKeys.detail(bountyId)); } to properly refresh server state.
113-117: TODO comment indicates incomplete implementation fortotalSlotsOccupied.The comment acknowledges that
totalSlotsOccupiedshould be decremented but isn't implemented. Per issue#205, removing a contributor should "decrementtotalSlotsOccupied(visible removal and slot count update)."Would you like me to help implement the
totalSlotsOccupieddecrement, or should this be tracked as a follow-up issue?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/use-bounty-application.ts` around lines 113 - 117, The TODO notes that totalSlotsOccupied isn't decremented when removing a contributor; update the same update patch that builds the new state (the object containing contributorProgress: contributorProgress.filter((c)=>c.userId!==contributorId)) to also adjust totalSlotsOccupied: read the current totalSlotsOccupied value and, if it's a number, set totalSlotsOccupied to Math.max(0, totalSlotsOccupied - 1) (preserving immutability), otherwise leave it undefined or omit it; ensure you reference the same update object where contributorProgress is modified so the removal and slot decrement happen atomically.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/bounty-detail/model4-maintainer-dashboard.tsx`:
- Around line 60-71: Add onError handlers to the mutation calls so failures
surface to users; for each mutate invocation (e.g., releasePayment.mutate in
handleReleasePayment and the other mutate calls referenced around the same area
like approveContribution.mutate and rejectContribution.mutate), pass an onError
callback that calls toast.error with a clear message that includes the error
message or a fallback (e.g., `toast.error(\`Failed to release payment:
${error?.message ?? 'unknown error'}\`)`). Ensure the onError is paired with the
existing onSuccess handlers and uses the mutation-specific context
(contributor.userName or relevant IDs) to make the toast informative.
- Around line 104-115: handleSendMessage currently calls sendMessage.mutate
without an onError handler; update handleSendMessage to pass an onError callback
to sendMessage.mutate (alongside the existing onSuccess) that shows an error
toast (e.g., toast.error with a descriptive message and optional error.message)
and keeps the message UI open or resets state appropriately. Locate the
sendMessage.mutate call inside the handleSendMessage function and add the
onError option to mirror other mutation handlers in this file.
- Around line 268-276: The Advance button currently renders only Loader2 when
advanceContributor.isPending for the contributor, removing the "Advance" text
and ArrowRight icon; update the JSX in the Advance button (the block using
advanceContributor, contributor.userId, Loader2 and ArrowRight) to always render
the "Advance" label and ArrowRight icon and conditionally render Loader2
alongside them when advanceContributor.isPending (e.g., show Loader2
before/after the label and keep the label+icon visible), preserving the existing
conditional check on advanceContributor.variables?.contributorId to target the
correct contributor.
In `@hooks/use-bounty-application.ts`:
- Around line 83-88: The optimistic mutation handlers (advanceContributor and
removeContributor) currently only rollback via queryClient.setQueryData in their
onError callbacks, leaving users uninformed; update both mutation definitions to
add an onError that (1) rolls back as now using context.previous (keep
queryClient.setQueryData(bountyKeys.detail(bountyId), context.previous)), and
(2) triggers a user-facing error toast/message with the error details (use the
mutation onError params to extract the error) so the UI both reverts and shows a
clear failure message to the user; reference the existing onError blocks for
these mutations and reuse bountyId, bountyKeys.detail, and the context.previous
logic.
- Around line 60-78: The cache update path assumes there is a next milestone but
doesn't handle the case when milestoneIndex === milestones.length - 1, so the
mutation reports success with no state change; update the mutation to guard
against already-at-final-milestone by checking milestoneIndex against
milestones.length - 1 (using the same variables: milestoneIndex, milestones,
contributorIndex, contributorProgress) and either return early or throw an error
from the mutationFn so the success path (which calls queryClient.setQueryData on
bountyKeys.detail(bountyId)) and the success toast are not executed; ensure the
guard is placed before the code that constructs newProgress and calls
queryClient.setQueryData to prevent misleading UX.
---
Nitpick comments:
In `@hooks/use-bounty-application.ts`:
- Around line 32-88: The optimistic update in advanceContributor (and similarly
in removeContributor) never reconciles with the server; add an onSettled handler
to the useMutation config for advanceContributor that calls
queryClient.invalidateQueries(bountyKeys.detail(bountyId)) (and optionally
queryClient.refetchQueries) to ensure the bounty detail cache is refetched after
success or error; locate the useMutation for advanceContributor (and the
removeContributor mutation) and append an onSettled: (_data, _error, _vars,
_context) => { queryClient.invalidateQueries(bountyKeys.detail(bountyId)); } to
properly refresh server state.
- Around line 113-117: The TODO notes that totalSlotsOccupied isn't decremented
when removing a contributor; update the same update patch that builds the new
state (the object containing contributorProgress:
contributorProgress.filter((c)=>c.userId!==contributorId)) to also adjust
totalSlotsOccupied: read the current totalSlotsOccupied value and, if it's a
number, set totalSlotsOccupied to Math.max(0, totalSlotsOccupied - 1)
(preserving immutability), otherwise leave it undefined or omit it; ensure you
reference the same update object where contributorProgress is modified so the
removal and slot decrement happen atomically.
🪄 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: d68301e8-8fd6-473d-99a3-57712b9a2bfb
📒 Files selected for processing (3)
components/bounty-detail/bounty-detail-client.tsxcomponents/bounty-detail/model4-maintainer-dashboard.tsxhooks/use-bounty-application.ts
| const handleReleasePayment = (contributor: ContributorProgress) => { | ||
| releasePayment.mutate( | ||
| { | ||
| contributorId: contributor.userId, | ||
| milestoneId: contributor.currentMilestoneId, | ||
| }, | ||
| { | ||
| onSuccess: () => | ||
| toast.success(`Payment released for ${contributor.userName}`), | ||
| }, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Missing onError callbacks on mutation calls.
The mutate calls only handle onSuccess but don't handle onError. If the hook-level onError doesn't show a toast (which it currently doesn't), users won't know their action failed.
Add onError callbacks to show error toasts
const handleReleasePayment = (contributor: ContributorProgress) => {
releasePayment.mutate(
{
contributorId: contributor.userId,
milestoneId: contributor.currentMilestoneId,
},
{
onSuccess: () =>
toast.success(`Payment released for ${contributor.userName}`),
+ onError: () =>
+ toast.error(`Failed to release payment for ${contributor.userName}`),
},
);
};
const handleAdvance = (contributor: ContributorProgress) => {
advanceContributor.mutate(
{ contributorId: contributor.userId },
{
onSuccess: () =>
toast.success(`${contributor.userName} advanced to next milestone`),
+ onError: () =>
+ toast.error(`Failed to advance ${contributor.userName}`),
},
);
};
const handleRemove = (contributor: ContributorProgress) => {
removeContributor.mutate(
{ contributorId: contributor.userId },
{
onSuccess: () =>
toast.success(`${contributor.userName} removed from bounty`),
+ onError: () =>
+ toast.error(`Failed to remove ${contributor.userName}`),
},
);
};Also applies to: 73-81, 83-91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 60 -
71, Add onError handlers to the mutation calls so failures surface to users; for
each mutate invocation (e.g., releasePayment.mutate in handleReleasePayment and
the other mutate calls referenced around the same area like
approveContribution.mutate and rejectContribution.mutate), pass an onError
callback that calls toast.error with a clear message that includes the error
message or a fallback (e.g., `toast.error(\`Failed to release payment:
${error?.message ?? 'unknown error'}\`)`). Ensure the onError is paired with the
existing onSuccess handlers and uses the mutation-specific context
(contributor.userName or relevant IDs) to make the toast informative.
| const handleSendMessage = () => { | ||
| if (!selectedContributor || !messageText.trim()) return; | ||
| sendMessage.mutate( | ||
| { contributorId: selectedContributor.userId, message: messageText }, | ||
| { | ||
| onSuccess: () => { | ||
| toast.success(`Message sent to ${selectedContributor.userName}`); | ||
| setIsMessageOpen(false); | ||
| }, | ||
| }, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Missing onError handler for sendMessage mutation.
Similar to other handlers, handleSendMessage should handle errors so users know if message delivery failed.
Add onError callback
const handleSendMessage = () => {
if (!selectedContributor || !messageText.trim()) return;
sendMessage.mutate(
{ contributorId: selectedContributor.userId, message: messageText },
{
onSuccess: () => {
toast.success(`Message sent to ${selectedContributor.userName}`);
setIsMessageOpen(false);
},
+ onError: () => {
+ toast.error(`Failed to send message to ${selectedContributor.userName}`);
+ },
},
);
};📝 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.
| const handleSendMessage = () => { | |
| if (!selectedContributor || !messageText.trim()) return; | |
| sendMessage.mutate( | |
| { contributorId: selectedContributor.userId, message: messageText }, | |
| { | |
| onSuccess: () => { | |
| toast.success(`Message sent to ${selectedContributor.userName}`); | |
| setIsMessageOpen(false); | |
| }, | |
| }, | |
| ); | |
| }; | |
| const handleSendMessage = () => { | |
| if (!selectedContributor || !messageText.trim()) return; | |
| sendMessage.mutate( | |
| { contributorId: selectedContributor.userId, message: messageText }, | |
| { | |
| onSuccess: () => { | |
| toast.success(`Message sent to ${selectedContributor.userName}`); | |
| setIsMessageOpen(false); | |
| }, | |
| onError: () => { | |
| toast.error(`Failed to send message to ${selectedContributor.userName}`); | |
| }, | |
| }, | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 104 -
115, handleSendMessage currently calls sendMessage.mutate without an onError
handler; update handleSendMessage to pass an onError callback to
sendMessage.mutate (alongside the existing onSuccess) that shows an error toast
(e.g., toast.error with a descriptive message and optional error.message) and
keeps the message UI open or resets state appropriately. Locate the
sendMessage.mutate call inside the handleSendMessage function and add the
onError option to mirror other mutation handlers in this file.
| {advanceContributor.isPending && | ||
| advanceContributor.variables?.contributorId === | ||
| contributor.userId ? ( | ||
| <Loader2 className="size-3 mr-1.5 animate-spin" /> | ||
| ) : ( | ||
| <> | ||
| Advance [Coming soon]{" "} | ||
| <ArrowRight className="size-3 ml-1.5" /> | ||
| Advance <ArrowRight className="size-3 ml-1.5" /> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Advance button label missing during loading state.
When advanceContributor.isPending is true for this contributor, only the Loader2 spinner is rendered. The "Advance" text and arrow icon are lost, unlike the other buttons that keep their labels during loading.
Keep label visible during loading
>
- {advanceContributor.isPending &&
- advanceContributor.variables?.contributorId ===
- contributor.userId ? (
+ {advanceContributor.isPending &&
+ advanceContributor.variables?.contributorId ===
+ contributor.userId && (
<Loader2 className="size-3 mr-1.5 animate-spin" />
- ) : (
- <>
- Advance <ArrowRight className="size-3 ml-1.5" />
- </>
)}
+ Advance <ArrowRight className="size-3 ml-1.5" />
</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.
| {advanceContributor.isPending && | |
| advanceContributor.variables?.contributorId === | |
| contributor.userId ? ( | |
| <Loader2 className="size-3 mr-1.5 animate-spin" /> | |
| ) : ( | |
| <> | |
| Advance [Coming soon]{" "} | |
| <ArrowRight className="size-3 ml-1.5" /> | |
| Advance <ArrowRight className="size-3 ml-1.5" /> | |
| </> | |
| )} | |
| {advanceContributor.isPending && | |
| advanceContributor.variables?.contributorId === | |
| contributor.userId && ( | |
| <Loader2 className="size-3 mr-1.5 animate-spin" /> | |
| )} | |
| Advance <ArrowRight className="size-3 ml-1.5" /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 268 -
276, The Advance button currently renders only Loader2 when
advanceContributor.isPending for the contributor, removing the "Advance" text
and ArrowRight icon; update the JSX in the Advance button (the block using
advanceContributor, contributor.userId, Loader2 and ArrowRight) to always render
the "Advance" label and ArrowRight icon and conditionally render Loader2
alongside them when advanceContributor.isPending (e.g., show Loader2
before/after the label and keep the label+icon visible), preserving the existing
conditional check on advanceContributor.variables?.contributorId to target the
correct contributor.
| if (milestoneIndex >= 0 && milestoneIndex < milestones.length - 1) { | ||
| const nextMilestone = milestones[milestoneIndex + 1]; | ||
| const newProgress = [...contributorProgress]; | ||
| newProgress[contributorIndex] = { | ||
| ...newProgress[contributorIndex], | ||
| currentMilestoneId: nextMilestone.id, | ||
| }; | ||
|
|
||
| queryClient.setQueryData<ExtendedBountyQuery>( | ||
| bountyKeys.detail(bountyId), | ||
| { | ||
| ...previous, | ||
| bounty: { | ||
| ...previous.bounty, | ||
| contributorProgress: newProgress, | ||
| }, | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
No guard when contributor is already at the last milestone.
If a contributor is on the final milestone (milestoneIndex === milestones.length - 1), the condition at line 60 prevents the cache update, but the mutation still "succeeds" with a toast saying the contributor advanced. This creates a confusing UX where the user sees success but nothing changes.
Consider throwing or returning early in mutationFn
const advanceContributor = useMutation({
- mutationFn: async ({ contributorId }: { contributorId: string }) => {
+ mutationFn: async ({ contributorId }: { contributorId: string }) => {
+ // Check if already at last milestone before making the call
+ const current = queryClient.getQueryData<ExtendedBountyQuery>(
+ bountyKeys.detail(bountyId),
+ );
+ if (current?.bounty) {
+ const progress = current.bounty.contributorProgress || [];
+ const milestones = current.bounty.milestones || [];
+ const contributor = progress.find((c) => c.userId === contributorId);
+ if (contributor) {
+ const idx = milestones.findIndex((m) => m.id === contributor.currentMilestoneId);
+ if (idx >= milestones.length - 1) {
+ throw new Error("Contributor is already at the final milestone");
+ }
+ }
+ }
await delay(1000);
return { contributorId };
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/use-bounty-application.ts` around lines 60 - 78, The cache update path
assumes there is a next milestone but doesn't handle the case when
milestoneIndex === milestones.length - 1, so the mutation reports success with
no state change; update the mutation to guard against already-at-final-milestone
by checking milestoneIndex against milestones.length - 1 (using the same
variables: milestoneIndex, milestones, contributorIndex, contributorProgress)
and either return early or throw an error from the mutationFn so the success
path (which calls queryClient.setQueryData on bountyKeys.detail(bountyId)) and
the success toast are not executed; ensure the guard is placed before the code
that constructs newProgress and calls queryClient.setQueryData to prevent
misleading UX.
| onError: (_err, _vars, context) => { | ||
| if (context?.previous) { | ||
| queryClient.setQueryData(bountyKeys.detail(bountyId), context.previous); | ||
| } | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Missing onError callback for user feedback on mutation failures.
Both advanceContributor and removeContributor silently rollback on error without notifying the user. When optimistic updates fail, the UI reverts but the user has no indication that their action didn't succeed on the server.
Proposed fix to add error toasts
+import { toast } from "sonner";
+
// In advanceContributor mutation:
onError: (_err, _vars, context) => {
if (context?.previous) {
queryClient.setQueryData(bountyKeys.detail(bountyId), context.previous);
}
+ toast.error("Failed to advance contributor. Please try again.");
},
// In removeContributor mutation:
onError: (_err, _vars, context) => {
if (context?.previous) {
queryClient.setQueryData(bountyKeys.detail(bountyId), context.previous);
}
+ toast.error("Failed to remove contributor. Please try again.");
},Also applies to: 123-128
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/use-bounty-application.ts` around lines 83 - 88, The optimistic
mutation handlers (advanceContributor and removeContributor) currently only
rollback via queryClient.setQueryData in their onError callbacks, leaving users
uninformed; update both mutation definitions to add an onError that (1) rolls
back as now using context.previous (keep
queryClient.setQueryData(bountyKeys.detail(bountyId), context.previous)), and
(2) triggers a user-facing error toast/message with the error details (use the
mutation onError params to extract the error) so the UI both reverts and shows a
clear failure message to the user; reference the existing onError blocks for
these mutations and reuse bountyId, bountyKeys.detail, and the context.previous
logic.
|
@Jopsan-gm 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! 🚀 |
|
Hello @Benjtalkshow PR ready! ;) |
A few things to fix before this can merge. This PR duplicates PR #263's work for issue #211. The Three actions don't fully meet the acceptance criteria. Release Payment only invalidates without an escrow update. Remove filters Two code quality items: Last thing: there's significant Prettier reformatting noise across 10+ unrelated files ( |
…eaning up PR noise
|
Hi @Jopsan-gm One last thing: Once that's reverted, this is ready. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 62-63: Add persist-credentials: false to both checkout steps that
use actions/checkout@v4 to avoid leaking the default GITHUB_TOKEN to later
steps; locate the steps that include "uses: actions/checkout@v4" and add the
key-value pair persist-credentials: false under each step so credentials are not
persisted for subsequent workflow actions.
- Line 63: The workflow currently uses floating major tags and leaves checkout
credentials persisted; update the specified actions to pinned commit SHAs and
explicitly disable persisted credentials for checkout: replace uses:
actions/checkout@v4 with a pinned commit SHA and add with: persist-credentials:
false to the checkout step, and similarly pin actions/setup-node@v4,
pnpm/action-setup@v4, actions/cache@v4, and actions/upload-artifact@v4 to their
corresponding commit SHAs (use the official repository commit SHAs for each
action) so the CI references immutable versions and credentials are not stored
in git config.
In `@hooks/use-bounty-application.ts`:
- Line 542: Remove the debug console.log that prints raw user message in the
useSendMessage code path: delete or replace the line
console.log(`[useSendMessage] Recorded message for bountyId ${bountyId}:
contributorId=${contributorId}, message="${message}"`); so user-authored message
text is not logged; if telemetry is needed, log only non-sensitive identifiers
(e.g., bountyId and contributorId) or rely on the existing recordedMessages
storage instead—refer to useSendMessage, recordedMessages, bountyId,
contributorId, and message to locate the statement.
- Around line 355-357: The code computes milestonesCount using
"previous?.bounty?.milestones?.length ?? 1" which does not guard against an
empty array (length 0) and can cause division by zero for amountToRelease;
update both occurrences (the calculation of milestonesCount and the identical
logic in onMutate) to treat a zero length as fallback to 1—e.g., replace the
nullish fallback with a truthy fallback or use Math.max to ensure
milestonesCount is at least 1 (referencing totalAmount, milestonesCount,
amountToRelease and the onMutate handler to locate both places).
🪄 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: 4697e821-3068-4aa7-b453-5bb849a111f2
📒 Files selected for processing (5)
.github/workflows/ci.ymlcomponents/bounty-detail/bounty-detail-client.tsxcomponents/bounty-detail/model4-maintainer-dashboard.tsxhooks/use-bounty-application.tslib/services/escrow.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/bounty-detail/bounty-detail-client.tsx
| - name: Checkout repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print relevant section of the workflow
FILE=".github/workflows/ci.yml"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
# Show around the referenced lines
nl -ba "$FILE" | sed -n '45,95p'Repository: boundlessfi/bounties
Length of output: 108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
test -f "$FILE"
echo "== Preview lines 55-80 =="
cat -n "$FILE" | sed -n '55,80p'
echo
echo "== Find all actions/checkout steps =="
rg -n "uses:\s*actions/checkout@|actions/checkout@" "$FILE" || true
echo
echo "== Find persist-credentials settings =="
rg -n "persist-credentials" "$FILE" || trueRepository: boundlessfi/bounties
Length of output: 1026
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== Preview lines 1-40 =="
cat -n "$FILE" | sed -n '1,40p'
echo
echo "== Find all persist-credentials settings =="
rg -n "persist-credentials" "$FILE" || trueRepository: boundlessfi/bounties
Length of output: 1286
Disable persisted checkout credentials in CI (persist-credentials: false).
Both actions/checkout@v4 steps (lines 20 and 63) lack persist-credentials: false, which leaves the default token configured for later steps.
🛡️ Suggested fix
- name: Checkout repository
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 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.
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| - name: Checkout repository | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 62-63: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 63-63: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 62 - 63, Add persist-credentials:
false to both checkout steps that use actions/checkout@v4 to avoid leaking the
default GITHUB_TOKEN to later steps; locate the steps that include "uses:
actions/checkout@v4" and add the key-value pair persist-credentials: false under
each step so credentials are not persisted for subsequent workflow actions.
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
ls -l "$FILE"
echo
echo "== Relevant uses lines (with numbers) =="
nl -ba "$FILE" | sed -n '40,140p'
echo
echo "== rg for uses: actions/ =="
rg -n "uses:\s*actions/|uses:\s*pnpm/" "$FILE" || trueRepository: boundlessfi/bounties
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
ls -l "$FILE"
echo
echo "== Relevant uses lines (with numbers) =="
nl -ba "$FILE" | sed -n '40,140p'
echo
echo "== rg for uses: actions/ =="
rg -n "uses:\s*actions/|uses:\s*pnpm/" "$FILE" || trueRepository: boundlessfi/bounties
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
ls -l "$FILE"
echo
echo "== Relevant uses lines (with numbers) =="
nl -ba "$FILE" | sed -n '40,140p'
echo
echo "== rg for uses: actions/ =="
rg -n "uses:\s*actions/|uses:\s*pnpm/" "$FILE" || trueRepository: boundlessfi/bounties
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,200p'Repository: boundlessfi/bounties
Length of output: 162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
ls -l "$FILE"
echo
echo "== Line-numbered excerpt (40-140) =="
awk '{printf "%d\t%s\n", NR, $0}' "$FILE" | sed -n '40,140p'
echo
echo "== rg uses: actions/ or pnpm/ =="
rg -n "uses:\s*actions/|uses:\s*pnpm/" "$FILE" || true
echo
echo "== rg for persist-credentials in checkout steps =="
rg -n "persist-credentials" "$FILE" || trueRepository: boundlessfi/bounties
Length of output: 3938
🌐 Web query:
GitHub Actions actions/checkout persist-credentials default value true and description
💡 Result:
For actions/checkout, the input persist-credentials has default value true, and it controls whether the auth token / SSH key is configured in the local git config. Default value: true [1] Description: “Whether to configure the token or SSH key with the local git config” [1] Additional context from the action README: the auth token is persisted in the local git config so scripts can run authenticated git commands, and the token is removed during post-job cleanup; you can opt out with persist-credentials: false. [2]
Citations:
- 1: https://github.com/actions/checkout/blob/72f2cec99f417b1a1c5e2e88945068983b7965f9/action.yml
- 2: https://github.com/actions/checkout/?tab=readme-ov-file
🌐 Web query:
actions/checkout persist-credentials defaults to true
💡 Result:
For actions/checkout, the persist-credentials input defaults to true (i.e., the token is persisted locally) unless you explicitly set persist-credentials: false.[1][2] The action documentation states that “The auth token is persisted in the local git config… Set persist-credentials: false to opt-out.”[1] Security discussions in the actions/checkout issue tracker also describe the parameter’s default as true and that you must explicitly pass persist-credentials: false to prevent persistence.[2]
Citations:
- 1: https://github.com/actions/checkout/?tab=readme-ov-file
- 2: Remove
persist-credentialsor change the default tofalseactions/checkout#485
🌐 Web query:
"persist-credentials" actions/checkout defaults to true
💡 Result:
Yes, the persist-credentials input for the actions/checkout GitHub Action defaults to true [1][2][3]. When this setting is enabled (the default), the action configures the local Git environment with the authentication token (such as the GITHUB_TOKEN) used to fetch the repository [1][2][3]. This allows subsequent steps in the same job to execute authenticated Git commands—such as git push or git pull—without requiring further manual configuration [1][3]. The token is removed from the local Git configuration during the post-job cleanup phase [1][2][3]. Security practitioners often recommend setting persist-credentials to false in workflows that do not require Git write access (e.g., build or test jobs) to adhere to the principle of least privilege [4][5][6]. This prevents the authentication token from being available to other steps in the job, which can reduce the impact if a subsequent step is compromised [4][6]. To disable this behavior, use the following syntax in your workflow file [5]: - uses: actions/checkout@v4 with: persist-credentials: false
Citations:
- 1: actions/checkout@c170eef
- 2: https://github.com/actions/checkout/tree/refs/heads/main
- 3: https://github.com/actions/checkout/?tab=readme-ov-file
- 4: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 5: https://yossarian.net/til/post/actions-checkout-can-leak-github-credentials/
- 6: Add persist-credentials: false to GitHub Actions checkout steps aquasecurity/trivy#10305
🌐 Web query:
actions/checkout@v4 persist-credentials documentation default
💡 Result:
In the actions/checkout@v4 action, the default value for the persist-credentials input is true [1][2][3]. When this setting is enabled (the default), the action configures the repository's local git configuration to include the authentication token or SSH key used for the checkout [1][4]. This allows subsequent steps in your workflow to run authenticated git commands (such as git push) without needing to manually re-configure credentials [1][5]. The token is automatically removed during the post-job cleanup phase [1][2][4]. If you wish to prevent this behavior—for example, to enhance security by ensuring the credentials are not available to subsequent steps in the workflow—you must explicitly set persist-credentials to false in your workflow YAML [1][5]: steps: - uses: actions/checkout@v4 with: persist-credentials: false
Citations:
- 1: https://github.com/actions/checkout/tree/v4
- 2: https://github.com/actions/checkout
- 3: https://github.com/actions/checkout/tree/db0cee9a514becbbd4a101a5fbbbf47865ee316c
- 4: https://github.com/actions/checkout/blob/85e6279cec87321a52edac9c87bce653a07cf6c2/README.md
- 5: https://yossarian.net/til/post/actions-checkout-can-leak-github-credentials/
Pin GitHub Actions to commit SHAs and set checkout token persistence explicitly.
In .github/workflows/ci.yml, the following steps still use major tags instead of commit SHAs: actions/checkout@v4 (line 63), actions/setup-node@v4 (line 66), pnpm/action-setup@v4 (line 71), actions/cache@v4 (line 76), and actions/upload-artifact@v4 (line 108). This weakens supply-chain guarantees.
Also, actions/checkout@v4 (line 63) does not set with: persist-credentials: false; persist-credentials defaults to true, so credentials are persisted locally in git config unless opted out.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 62-63: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 63-63: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 63, The workflow currently uses floating
major tags and leaves checkout credentials persisted; update the specified
actions to pinned commit SHAs and explicitly disable persisted credentials for
checkout: replace uses: actions/checkout@v4 with a pinned commit SHA and add
with: persist-credentials: false to the checkout step, and similarly pin
actions/setup-node@v4, pnpm/action-setup@v4, actions/cache@v4, and
actions/upload-artifact@v4 to their corresponding commit SHAs (use the official
repository commit SHAs for each action) so the CI references immutable versions
and credentials are not stored in git config.
| const totalAmount = previous?.bounty?.rewardAmount ?? 100; | ||
| const milestonesCount = previous?.bounty?.milestones?.length ?? 1; | ||
| const amountToRelease = totalAmount / milestonesCount; |
There was a problem hiding this comment.
Empty milestones array causes division by zero.
?? 1 only guards null/undefined, not an empty array. If milestones is [], length is 0 and 0 ?? 1 evaluates to 0, so amountToRelease becomes Infinity. The same pattern repeats in onMutate (Line 373). Use || to also cover the empty/zero case.
🐛 Proposed fix (apply to both Line 356 and Line 373)
- const milestonesCount = previous?.bounty?.milestones?.length ?? 1;
+ const milestonesCount = previous?.bounty?.milestones?.length || 1;📝 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.
| const totalAmount = previous?.bounty?.rewardAmount ?? 100; | |
| const milestonesCount = previous?.bounty?.milestones?.length ?? 1; | |
| const amountToRelease = totalAmount / milestonesCount; | |
| const totalAmount = previous?.bounty?.rewardAmount ?? 100; | |
| const milestonesCount = previous?.bounty?.milestones?.length || 1; | |
| const amountToRelease = totalAmount / milestonesCount; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/use-bounty-application.ts` around lines 355 - 357, The code computes
milestonesCount using "previous?.bounty?.milestones?.length ?? 1" which does not
guard against an empty array (length 0) and can cause division by zero for
amountToRelease; update both occurrences (the calculation of milestonesCount and
the identical logic in onMutate) to treat a zero length as fallback to 1—e.g.,
replace the nullish fallback with a truthy fallback or use Math.max to ensure
milestonesCount is at least 1 (referencing totalAmount, milestonesCount,
amountToRelease and the onMutate handler to locate both places).
| timestamp: new Date().toISOString(), | ||
| }); | ||
|
|
||
| console.log(`[useSendMessage] Recorded message for bountyId ${bountyId}: contributorId=${contributorId}, message="${message}"`); |
There was a problem hiding this comment.
Drop the console.log of raw message content.
This logs user-authored message text to the console, which is a debug artifact and a potential privacy concern. The message is already recorded in recordedMessages.
🧹 Proposed fix
- console.log(`[useSendMessage] Recorded message for bountyId ${bountyId}: contributorId=${contributorId}, message="${message}"`);
return { contributorId, message };📝 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.
| console.log(`[useSendMessage] Recorded message for bountyId ${bountyId}: contributorId=${contributorId}, message="${message}"`); | |
| return { contributorId, message }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/use-bounty-application.ts` at line 542, Remove the debug console.log
that prints raw user message in the useSendMessage code path: delete or replace
the line console.log(`[useSendMessage] Recorded message for bountyId
${bountyId}: contributorId=${contributorId}, message="${message}"`); so
user-authored message text is not logged; if telemetry is needed, log only
non-sensitive identifiers (e.g., bountyId and contributorId) or rely on the
existing recordedMessages storage instead—refer to useSendMessage,
recordedMessages, bountyId, contributorId, and message to locate the statement.
|
Hey @Benjtalkshow, thanks for the review! I just pushed the latest updates. types/bounty.ts has been completely reverted to match upstream, and I also went ahead and resolved the recent merge conflicts with the main branch. Everything should be ready to merge now. Let me know if there's anything else! |
PR #265 dropped --with-deps from the Playwright install step. Without it, fresh CI runners that don't have Chromium's system libs (fonts, shared libraries) cached will fail to launch the browser. Putting it back.
2 participants
Title feat: implement maintainer dashboard actions for multi-winner milestone bounties
Closes
Closes #205
Description
This PR implements the requested actions for the Model 4 (Multi-Winner Milestone) Maintainer Dashboard, transitioning it from a static UI to a fully functional interface with optimistic updates.
Changes Made:
handleActionstub with dedicated mutation hooks using@tanstack/react-query(releasePayment,advanceContributor,removeContributor,sendMessage).sonnertoasts to provide immediate success feedback after an action is executed.[Coming soon]suffix from action buttons.Dialogcomponents for the View Submissions and Message actions.bountyIdappropriately to ensure the mutations target the correct bounty scope.contributorProgressfields.Screenshots
Summary by CodeRabbit
New Features
Chores