Conversation
✅ Multi-LLM Code Review — APPROVEAggregated Score: 6.7/10 (from 3 models) Model Verdicts
Category Scores (Averaged)
SummaryGPT-5.4: This submission shows solid backend-oriented work and some thoughtful validation improvements, but it does not yet read as a fully complete bounty creation wizard implementation. The main concerns are spec alignment, end-to-end completeness, and whether the submitted changes are fully representative of the claimed 7-step workflow. Issues
Suggestions
Reviewed by SolFoundry Multi-LLM Pipeline: GPT-5.4, Gemini 2.5 Pro, Grok 4 Review profile: frontend — scoring weights adjusted for this domain |
📝 WalkthroughWalkthroughA new route Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/app/models/bounty.py (1)
160-205:⚠️ Potential issue | 🟠 MajorDeadline validation is only enforced in the UI, not the API.
deadlineis a baredatetimein both request models, and the validators in this file never check that it lies in the future. A caller can POST or PATCH a past timestamp directly to/api/bounties, creating an immediately-expired bounty even though step 5 rejects that input in the wizard.
As per coding guidelines,backend/**: Python FastAPI backend. Analyze thoroughly: Input validation and SQL injection vectors; API contract consistency with spec.Also applies to: 214-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/models/bounty.py` around lines 160 - 205, Add validation to ensure the deadline (the deadline field) is either None or a future timestamp: implement a validator (e.g., `@field_validator`("deadline") or a `@model_validator`(mode="after") named validate_deadline_future) that compares the provided datetime to the current UTC time (use datetime.now(timezone.utc) and require timezone-aware or convert appropriately) and raise ValueError if deadline <= now; add the same validator to the request models used for creating/updating bounties (e.g., the CreateBountyRequest/UpdateBountyRequest models in this file) so API-level POST/PATCH cannot accept past deadlines.backend/app/services/bounty_service.py (2)
95-116:⚠️ Potential issue | 🔴 CriticalPublish is still anonymous and local-only.
This path just copies
data.created_byinto_bounty_storeand returns a generated UUID. There is no authenticated principal in the method signature or call path, and nothing here creates the GitHub issue that step 7 promises. The wizard can therefore report a successful publish for an anonymous or spoofed creator even though no backing issue exists.
As per coding guidelines,backend/**: Python FastAPI backend. Analyze thoroughly: Authentication/authorization gaps; API contract consistency with spec.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/bounty_service.py` around lines 95 - 116, create_bounty currently trusts data.created_by and never creates the promised GitHub issue, allowing anonymous/spoofed publishes; change create_bounty to accept/obtain the authenticated principal (e.g., current_user) rather than using data.created_by, verify the principal is authorized for the bounty, call the GitHub issue creation helper (e.g., github_client.create_issue or IssueService.create_github_issue) and only persist to _bounty_store if the GitHub issue creation succeeds, populate the stored bounty's created_by and github_issue_url from the authenticated principal and the real issue URL, and on GitHub failure roll back/avoid storing and return an appropriate error; update the call sites and _to_bounty_response usage to ensure the response contains the generated UUID and the real github_issue_url.
196-212:⚠️ Potential issue | 🟠 MajorPATCH can persist a bounty that violates its tier reward cap.
BountyCreate.validate_reward_in_tier_range()only runs on creation inbackend/app/models/bounty.py, Lines 186-205. Here the partial payload is applied directly withsetattron Lines 209-210, so a T1 bounty can be patched fromreward_amount=50toreward_amount=900and remain in storage in an invalid state. Revalidate the merged record before saving.
As per coding guidelines,backend/**: Python FastAPI backend. Analyze thoroughly: Input validation and SQL injection vectors; API contract consistency with spec.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/bounty_service.py` around lines 196 - 212, The patch applies partial updates directly to the bounty instance with setattr (the loop that iterates updates and the subsequent bounty.updated_at assignment) without re-running creation-time validation, so tier reward caps can be violated; after merging updates into the bounty (or into a temporary dict/object) call the existing validation routine BountyCreate.validate_reward_in_tier_range (or the appropriate model validator) against the merged state and return an error (None, message) if validation fails before persisting; ensure you reference the status transition logic that uses BountyStatus and VALID_STATUS_TRANSITIONS remains unchanged and perform validation immediately after the update loop and before saving/updating the DB.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/__tests__/create-bounty-wizard.test.tsx`:
- Around line 533-536: The test currently only fills 'input-reward' and doesn't
exercise the deadline field or assert the serialized request body; update the
round-trip test to also populate the deadline field (e.g., using
screen.getByTestId('input-deadline') or the appropriate date-picker test id) and
any remaining publish fields (tier/category selects, required_skills and
requirements inputs or chips used by the wizard) before clicking the final
publish button, then strengthen the fetch assertion to parse/assert the request
body contains the expected JSON keys/values (tier, category, required_skills,
deadline, requirements, reward) rather than ignoring body; target the test
helpers and elements referenced by getByTestId ('step-reward', 'input-reward',
'input-deadline', plus the tier/category/skills test ids) and update the
expect(fetch).toHaveBeenCalledWith(...) check to validate JSON.stringify payload
contents.
- Around line 469-487: Add a new test that mocks the useAuth hook to return
isAuthenticated: false and verifies the unauthenticated branch: spyOn or
jest.mock the useAuth import used by renderWizard so it returns {
isAuthenticated: false }, call renderWizard(), then assert
screen.getByTestId('auth-required') is in the document and
screen.queryByTestId('create-bounty-page') / screen.queryByTestId('step-tier')
are not; name the test e.g. "renders auth gate when the user is not
authenticated" and restore the mock after the test.
- Around line 247-249: The tests type into getByTestId('input-description') but
never assert the rendered markdown, so add assertions after typing to verify the
markdown preview element (e.g., the component that renders the description
preview) contains the expected rendered content (plain text and expected HTML
like paragraphs/links) and to cover XSS: add a case that types a malicious
payload (e.g., "<img src=x onerror=alert(1)>" or "<script>") and assert the
preview sanitizes/escapes it (no script tag executed or present in innerHTML, no
alert/mock called). Update the same pattern in the other test locations
referenced (around lines 271-274, 513-514, 538-544) to assert both correct
markdown rendering and XSS prevention after typing into 'input-description'.
In `@frontend/src/hooks/useBountyWizard.ts`:
- Around line 73-79: loadDraft() currently parses localStorage JSON and blindly
casts it to WizardDraft, which can break later code that expects specific
arrays/strings; update loadDraft() to validate/normalize the parsed object by
either merging it with emptyDraft() or running a shape check before returning.
Specifically, after JSON.parse(raw) in loadDraft() (and the similar logic around
the other parsing block), create/return Object.assign(emptyDraft(), parsed) or
validate required fields (e.g., arrays/strings used by CreateBountyPage) and
fall back to emptyDraft() on mismatch so downstream calls (like .map() /
.join()) are safe; keep DRAFT_STORAGE_KEY and WizardDraft types but ensure
returned value always matches emptyDraft() shape.
In `@frontend/src/pages/CreateBountyPage.tsx`:
- Around line 103-137: The Title & Description step currently renders a raw
textarea (wiz.draft.description) and later shows the description in a <pre>, so
add a markdown preview renderer and integrate it into the wizard flow: replace
or augment the textarea in CreateBountyPage (case 1) to include a live Markdown
preview panel (or toggle) that renders wiz.draft.description using the project's
markdown renderer (e.g., the same renderer used elsewhere or a shared
MarkdownPreview component), and update the later preview step to render Markdown
instead of dumping into a <pre>; reference wiz.draft.description, wiz.setField,
the textarea element, and the later preview rendering code to locate where to
swap the raw text for the markdown-rendered output.
- Around line 28-31: The auth helper useAuth currently always returns
isAuthenticated: true, so update useAuth in CreateBountyPage to reflect real
auth state (e.g., read from AuthContext or invoke the existing GitHub/wallet
auth utilities) and ensure it verifies both GitHub and wallet presence; then
change the unauthenticated branch in the CreateBountyPage render (the guard that
currently becomes dead code) to actually execute when useAuth().isAuthenticated
is false—show the sign-in/authorize UI or redirect to the auth flow so
/bounties/create is protected by GitHub + wallet checks rather than being
hardcoded open.
- Around line 242-247: The datetime-local input is being converted with new
Date(...).toISOString() which shifts the user-entered local time to UTC; instead
keep the raw local datetime string in the wizard state and only convert to an
absolute UTC time at publish time. Change the onChange for the input with
id="deadline" to call wiz.setField('deadline', e.target.value || '') so
wiz.draft.deadline stores the naive "YYYY-MM-DDTHH:mm" the user entered, and
move any timezone-aware conversion logic into the publish/submit handler (the
code that reads wiz.draft.deadline to create the final deadline) where you can
parse the local components and produce a correct UTC ISO string.
In `@frontend/src/types/wizard.ts`:
- Around line 57-60: The request type in frontend/src/types/wizard.ts
incorrectly exposes server-derived fields; remove github_issue_url and
created_by from the client-side bounty creation type (or replace the current
type with a CreationRequest/ClientBounty type that omits those fields) and
introduce/use a separate server response type (e.g., BountyResponse or
ServerBounty) that includes github_issue_url and created_by. Update any
components or functions currently typing request payloads with the old type to
use the new client-only CreationRequest and ensure created_by is sourced from
the authenticated session and github_issue_url is read from server responses
only.
- Around line 49-54: CreateBountyPayload was widened incorrectly: change the
tier back to the shared WizardTier type and the category to the existing
WizardCategory union (or WizardCategory | undefined) so the API payload matches
frontend enums; update the interface named CreateBountyPayload in
frontend/src/types/wizard.ts and ensure any caller such as
useBountyWizard.publish() still serializes that interface directly so enum
validation is preserved at compile time.
- Around line 102-103: The DRAFT_STORAGE_KEY ('solfoundry_bounty_draft') is
global and causes cross-account draft leakage; change the storage strategy to be
per-authenticated-user by turning the single DRAFT_STORAGE_KEY into a scoped key
(e.g., export a DRAFT_STORAGE_KEY_PREFIX and a helper like
getDraftKey(userIdOrWallet) or getDraftStorageKey(walletAddress|userId)) and use
that helper wherever drafts are saved/loaded (replace direct uses of
DRAFT_STORAGE_KEY in draft persistence logic). Ensure the key includes a stable
per-user identifier (auth user id or connected wallet public key), update
save/load/delete calls to use getDraftKey(currentUserIdentifier), and clear or
namespace drafts on sign-out so drafts aren’t restored across different
accounts.
---
Outside diff comments:
In `@backend/app/models/bounty.py`:
- Around line 160-205: Add validation to ensure the deadline (the deadline
field) is either None or a future timestamp: implement a validator (e.g.,
`@field_validator`("deadline") or a `@model_validator`(mode="after") named
validate_deadline_future) that compares the provided datetime to the current UTC
time (use datetime.now(timezone.utc) and require timezone-aware or convert
appropriately) and raise ValueError if deadline <= now; add the same validator
to the request models used for creating/updating bounties (e.g., the
CreateBountyRequest/UpdateBountyRequest models in this file) so API-level
POST/PATCH cannot accept past deadlines.
In `@backend/app/services/bounty_service.py`:
- Around line 95-116: create_bounty currently trusts data.created_by and never
creates the promised GitHub issue, allowing anonymous/spoofed publishes; change
create_bounty to accept/obtain the authenticated principal (e.g., current_user)
rather than using data.created_by, verify the principal is authorized for the
bounty, call the GitHub issue creation helper (e.g., github_client.create_issue
or IssueService.create_github_issue) and only persist to _bounty_store if the
GitHub issue creation succeeds, populate the stored bounty's created_by and
github_issue_url from the authenticated principal and the real issue URL, and on
GitHub failure roll back/avoid storing and return an appropriate error; update
the call sites and _to_bounty_response usage to ensure the response contains the
generated UUID and the real github_issue_url.
- Around line 196-212: The patch applies partial updates directly to the bounty
instance with setattr (the loop that iterates updates and the subsequent
bounty.updated_at assignment) without re-running creation-time validation, so
tier reward caps can be violated; after merging updates into the bounty (or into
a temporary dict/object) call the existing validation routine
BountyCreate.validate_reward_in_tier_range (or the appropriate model validator)
against the merged state and return an error (None, message) if validation fails
before persisting; ensure you reference the status transition logic that uses
BountyStatus and VALID_STATUS_TRANSITIONS remains unchanged and perform
validation immediately after the update loop and before saving/updating the DB.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: adaf7707-8ed0-4866-a232-d7f7c0ae8fe0
📒 Files selected for processing (10)
backend/app/api/bounties.pybackend/app/main.pybackend/app/models/bounty.pybackend/app/services/bounty_service.pybackend/tests/test_bounties.pyfrontend/src/App.tsxfrontend/src/__tests__/create-bounty-wizard.test.tsxfrontend/src/hooks/useBountyWizard.tsfrontend/src/pages/CreateBountyPage.tsxfrontend/src/types/wizard.ts
| // Step 1: Title + description | ||
| await user.type(screen.getByTestId('input-title'), 'Fix the README typo'); | ||
| await user.type(screen.getByTestId('input-description'), 'Details here'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
The markdown preview is effectively untested.
Both end-to-end flows type into input-description, but neither one asserts anything about the rendered description content. That leaves the markdown renderer’s behavior, including the security-critical XSS case, completely uncovered.
As per coding guidelines, frontend/**: React/TypeScript frontend. Check: XSS prevention.
Also applies to: 271-274, 513-514, 538-544
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/__tests__/create-bounty-wizard.test.tsx` around lines 247 - 249,
The tests type into getByTestId('input-description') but never assert the
rendered markdown, so add assertions after typing to verify the markdown preview
element (e.g., the component that renders the description preview) contains the
expected rendered content (plain text and expected HTML like paragraphs/links)
and to cover XSS: add a case that types a malicious payload (e.g., "<img src=x
onerror=alert(1)>" or "<script>") and assert the preview sanitizes/escapes it
(no script tag executed or present in innerHTML, no alert/mock called). Update
the same pattern in the other test locations referenced (around lines 271-274,
513-514, 538-544) to assert both correct markdown rendering and XSS prevention
after typing into 'input-description'.
| // Step 4: Reward + deadline | ||
| expect(screen.getByTestId('step-reward')).toBeInTheDocument(); | ||
| await user.type(screen.getByTestId('input-reward'), '3000'); | ||
| await user.click(screen.getByTestId('btn-next')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Exercise the deadline field and serialized payload in the round-trip test.
Step 5 only fills input-reward, and the final fetch assertion ignores body. That means the date field plus the actual publish contract (tier, category, required_skills, deadline, and requirements serialization) can regress without failing CI.
As per coding guidelines, frontend/**: React/TypeScript frontend. Check: Integration with existing components.
Also applies to: 555-559
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/__tests__/create-bounty-wizard.test.tsx` around lines 533 - 536,
The test currently only fills 'input-reward' and doesn't exercise the deadline
field or assert the serialized request body; update the round-trip test to also
populate the deadline field (e.g., using screen.getByTestId('input-deadline') or
the appropriate date-picker test id) and any remaining publish fields
(tier/category selects, required_skills and requirements inputs or chips used by
the wizard) before clicking the final publish button, then strengthen the fetch
assertion to parse/assert the request body contains the expected JSON
keys/values (tier, category, required_skills, deadline, requirements, reward)
rather than ignoring body; target the test helpers and elements referenced by
getByTestId ('step-reward', 'input-reward', 'input-deadline', plus the
tier/category/skills test ids) and update the
expect(fetch).toHaveBeenCalledWith(...) check to validate JSON.stringify payload
contents.
frontend/src/types/wizard.ts
Outdated
| /** Payload sent to POST /api/bounties on publish (step 7). */ | ||
| export interface CreateBountyPayload { | ||
| tier: WizardTier; | ||
| title: string; | ||
| description: string; | ||
| category?: string; |
There was a problem hiding this comment.
Keep the publish request aligned with the shared enums.
CreateBountyPayload switches tier to 1 | 2 | 3 and widens category to plain string. The existing frontend bounty model already uses string tier values in frontend/src/types/bounty.ts:7, and this module already defines WizardCategory, so the new request shape removes the type checks that should protect the API boundary. If useBountyWizard.publish() serializes this interface directly, enum mismatches are only caught as 422s at runtime.
As per coding guidelines, frontend/**: React/TypeScript frontend. Check: Integration with existing components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/types/wizard.ts` around lines 49 - 54, CreateBountyPayload was
widened incorrectly: change the tier back to the shared WizardTier type and the
category to the existing WizardCategory union (or WizardCategory | undefined) so
the API payload matches frontend enums; update the interface named
CreateBountyPayload in frontend/src/types/wizard.ts and ensure any caller such
as useBountyWizard.publish() still serializes that interface directly so enum
validation is preserved at compile time.
frontend/src/types/wizard.ts
Outdated
| deadline?: string; | ||
| github_issue_url?: string; | ||
| created_by: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Do not advertise server-derived fields as caller input.
github_issue_url is created by publish, and created_by should come from the authenticated session. Even if the current backend ignores them, putting both on the request type tells future callers that they are legitimate client-supplied fields and weakens the trust boundary around bounty creation.
As per coding guidelines, frontend/**: React/TypeScript frontend. Check: Integration with existing components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/types/wizard.ts` around lines 57 - 60, The request type in
frontend/src/types/wizard.ts incorrectly exposes server-derived fields; remove
github_issue_url and created_by from the client-side bounty creation type (or
replace the current type with a CreationRequest/ClientBounty type that omits
those fields) and introduce/use a separate server response type (e.g.,
BountyResponse or ServerBounty) that includes github_issue_url and created_by.
Update any components or functions currently typing request payloads with the
old type to use the new client-only CreationRequest and ensure created_by is
sourced from the authenticated session and github_issue_url is read from server
responses only.
frontend/src/types/wizard.ts
Outdated
| /** localStorage key used for draft persistence. */ | ||
| export const DRAFT_STORAGE_KEY = 'solfoundry_bounty_draft'; |
There was a problem hiding this comment.
Draft persistence is not scoped per authenticated user.
A single solfoundry_bounty_draft key means switching GitHub or wallet accounts on the same browser restores the previous creator’s draft. That is a cross-account data leak and will create confusing state after sign-out or sign-in flows.
As per coding guidelines, frontend/**: React/TypeScript frontend. Check: Integration with existing components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/types/wizard.ts` around lines 102 - 103, The DRAFT_STORAGE_KEY
('solfoundry_bounty_draft') is global and causes cross-account draft leakage;
change the storage strategy to be per-authenticated-user by turning the single
DRAFT_STORAGE_KEY into a scoped key (e.g., export a DRAFT_STORAGE_KEY_PREFIX and
a helper like getDraftKey(userIdOrWallet) or
getDraftStorageKey(walletAddress|userId)) and use that helper wherever drafts
are saved/loaded (replace direct uses of DRAFT_STORAGE_KEY in draft persistence
logic). Ensure the key includes a stable per-user identifier (auth user id or
connected wallet public key), update save/load/delete calls to use
getDraftKey(currentUserIdentifier), and clear or namespace drafts on sign-out so
drafts aren’t restored across different accounts.
✅ Multi-LLM Code Review — APPROVEAggregated Score: 6.4/10 (from 3 models) Model Verdicts
Category Scores (Averaged)
SummaryGPT-5.4: This is a substantial feature submission with a clear wizard structure, routing integration, and a meaningful amount of test scaffolding. However, it still has important project-coherence and acceptance-criteria gaps, especially around real authentication behavior, draft robustness, and the completeness of the publish flow. Issues
Suggestions
Reviewed by SolFoundry Multi-LLM Pipeline: GPT-5.4, Gemini 2.5 Pro, Grok 4 Review profile: frontend — scoring weights adjusted for this domain |
ItachiDevv
force-pushed
the
fix/issue-24-bounty-creation-wizard
branch
from
e38e307 to
775e82a
Compare
✅ Multi-LLM Code Review — APPROVEAggregated Score: 6.0/10 (from 3 models) Model Verdicts
Category Scores (Averaged)
SummaryGPT-5.4: This submission introduces a substantial bounty creation wizard and does connect into the app in a meaningful way. However, it falls short of the full spec in a few important areas, and the test suite does not yet fully de-risk the more fragile branches of the flow. Issues
Suggestions
Reviewed by SolFoundry Multi-LLM Pipeline: GPT-5.4, Gemini 2.5 Pro, Grok 4 Review profile: frontend — scoring weights adjusted for this domain |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
frontend/src/pages/CreateBountyPage.tsx (2)
37-38:⚠️ Potential issue | 🟠 MajorThe gate still enforces wallet-only auth.
The wizard becomes reachable as soon as
useWalletConnection()reports a connected wallet, but Issue#24requires GitHub and wallet authentication. A wallet-connected user with no GitHub session can still open this page and publish.Also applies to: 57-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/CreateBountyPage.tsx` around lines 37 - 38, The page currently gates access using only useWalletConnection() (checking connected and publicKey) so a wallet-only user can reach the wizard; modify the guard in CreateBountyPage to require both wallet and GitHub auth by checking the GitHub session/state (e.g., your existing github session hook or isGithubAuthenticated flag) alongside connected/publicKey before rendering or allowing publish; if GitHub is missing, redirect (useNavigate nav) to the GitHub login/consent flow or show a blocking notice—update the checks around where connected, publicKey (addr) are used and also the publish handler at the later publish point to enforce the combined condition.
30-30:⚠️ Potential issue | 🟠 Major
type="date"is still being validated with UTC semantics.
new Date(d.deadline)parsesYYYY-MM-DDat midnight UTC, and the inputminis also derived fromtoISOString(). For users west of UTC, selecting “tomorrow” later in the day can already compare as not-in-the-future; users east of UTC can also see the minimum day drift. This needs local-date comparison, notDateparsing of date-only strings.Run the following to reproduce the current failure mode:
#!/bin/bash TZ=America/Los_Angeles node - <<'NODE' const now = new Date('2026-03-20T18:00:00-07:00'); const selected = '2026-03-21'; console.log({ minAttr: now.toISOString().split('T')[0], parsedSelected: new Date(selected).toISOString(), passesCurrentCheck: new Date(selected) > now, }); NODEAlso applies to: 64-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/CreateBountyPage.tsx` at line 30, The deadline validation and the input min use UTC semantics (new Date(d.deadline) and toISOString().split('T')[0]) which causes off-by-one across timezones; change both the validation and the min generation to use local-date semantics: parse the YYYY-MM-DD string into numeric year/month/day and construct a local-midnight Date via new Date(year, month-1, day) (or compare date components as YYYY/MM/DD integers) when checking d.deadline against today (use new Date() local year/month/day), and replace any toISOString().split('T')[0] min generation with a local date string built from today’s local year/month/day (pad month/day to two digits) so comparisons are done in local time; update the validation branch that references d.deadline (the step===4 check) and the code that sets the input min attribute accordingly.frontend/src/__tests__/create-bounty-wizard.test.tsx (1)
77-79: 🛠️ Refactor suggestion | 🟠 MajorThe publish test still leaves part of the request contract unasserted.
fdseedscategory,deadline, andrequirements, but the success assertion only checks title/tier/reward/skills/creator. That gap is why Line 54 infrontend/src/pages/CreateBountyPage.tsxcan currently dropcategorywithout failing CI, and it would also miss regressions in deadline or requirements serialization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/__tests__/create-bounty-wizard.test.tsx` around lines 77 - 79, The POST success test ("POSTs and navigates") currently checks title/tier/reward/required_skills/created_by but misses category, deadline and requirements from the seeded fd; update that test to parse the fetch request body (the spy/mock fetch used in the test) and add assertions that b.category === 'feature', b.deadline === '2099-12-31' and b.requirements (or b.requirements_raw depending on how CreateBountyPage serializes it) matches ['req1'] so missing-serialization regressions are caught; keep using the same spy/mock and restore behavior (spy.mockRestore()) after the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/App.tsx`:
- Around line 18-20: Router is missing a route for "/bounties" so navigations
from CreateBountyPage to "/bounties" get redirected to "/leaderboard"; add a
Route for path="/bounties" that renders the proper component (e.g., BountiesPage
or the bounty board component) alongside the existing Route entries, update the
route list that includes Route path="/bounties/create" and Route path="*" so the
new Route for "/bounties" appears before the catch-all Navigate, and ensure the
component name matches what CreateBountyPage expects when it redirects to
"/bounties".
In `@frontend/src/pages/CreateBountyPage.tsx`:
- Around line 54-55: The request body object b is missing the selected category
so draft.category is dropped; update the object construction in CreateBountyPage
(variable b) to include category: draft.category (and add the same property to
the equivalent POST payloads at the other occurrence around lines 63-65) so the
API and preview receive and persist the chosen category.
- Around line 10-13: TIER_INFO currently advertises USD ranges and the UI allows
any reward >= 0.01 while previewing "USDC", but the contract requires
tier-specific $FNDRY rules (including the T2 parameters from Issue `#24`); update
TIER_INFO to reflect the correct FNDRY ranges/labels for WizardTier 1–3, then
add enforcement in the bounty creation validation (the reward input validation
in CreateBountyPage and/or its onSubmit/validateReward handler) to reject values
outside the tier-specific min/max and to require the FNDRY token type, and
update the preview rendering (the reward preview component or JSX in
CreateBountyPage that currently shows "USDC") to display "FNDRY" and the
validated FNDRY amount. Ensure messages/labels use the same tier names from
TIER_INFO so UI and validation are consistent.
- Around line 19-20: loadDraft currently only checks title and requirements then
blindly spreads parsed object `p` into the live draft, which allows malformed
properties (e.g., required_skills null, invalid
tier/category/reward_amount/deadline) to crash later; update `loadDraft` to
fully validate the persisted draft shape against the expected BountyDraft fields
before merging: after JSON.parse, verify each field (title:string, requirements:
string[], required_skills: string[] (default []), tier: valid enum/string,
category: string, reward_amount: number, deadline: valid date/string) and only
copy/merge the whitelisted, type-checked properties into `{...EMPTY_DRAFT}` (or
return `EMPTY_DRAFT` if validation fails), using `dk` and preserving the
try/catch around parse to avoid throwing on malformed JSON.
- Around line 22-24: renderMarkdown currently preserves raw HTML and returns
HTML used with dangerouslySetInnerHTML, enabling XSS; fix by escaping
user-supplied HTML before or sanitizing the generated HTML before returning from
renderMarkdown (e.g., replace/encode <, >, &, " and ' on the input or run the
output through a sanitizer like DOMPurify) so any embedded tags/attributes
(e.g., <img onerror=...>) are neutralized; update the renderMarkdown function to
perform this escaping/sanitization and ensure the preview consumers that use
dangerouslySetInnerHTML receive the sanitized string.
---
Duplicate comments:
In `@frontend/src/__tests__/create-bounty-wizard.test.tsx`:
- Around line 77-79: The POST success test ("POSTs and navigates") currently
checks title/tier/reward/required_skills/created_by but misses category,
deadline and requirements from the seeded fd; update that test to parse the
fetch request body (the spy/mock fetch used in the test) and add assertions that
b.category === 'feature', b.deadline === '2099-12-31' and b.requirements (or
b.requirements_raw depending on how CreateBountyPage serializes it) matches
['req1'] so missing-serialization regressions are caught; keep using the same
spy/mock and restore behavior (spy.mockRestore()) after the assertions.
In `@frontend/src/pages/CreateBountyPage.tsx`:
- Around line 37-38: The page currently gates access using only
useWalletConnection() (checking connected and publicKey) so a wallet-only user
can reach the wizard; modify the guard in CreateBountyPage to require both
wallet and GitHub auth by checking the GitHub session/state (e.g., your existing
github session hook or isGithubAuthenticated flag) alongside connected/publicKey
before rendering or allowing publish; if GitHub is missing, redirect
(useNavigate nav) to the GitHub login/consent flow or show a blocking
notice—update the checks around where connected, publicKey (addr) are used and
also the publish handler at the later publish point to enforce the combined
condition.
- Line 30: The deadline validation and the input min use UTC semantics (new
Date(d.deadline) and toISOString().split('T')[0]) which causes off-by-one across
timezones; change both the validation and the min generation to use local-date
semantics: parse the YYYY-MM-DD string into numeric year/month/day and construct
a local-midnight Date via new Date(year, month-1, day) (or compare date
components as YYYY/MM/DD integers) when checking d.deadline against today (use
new Date() local year/month/day), and replace any toISOString().split('T')[0]
min generation with a local date string built from today’s local year/month/day
(pad month/day to two digits) so comparisons are done in local time; update the
validation branch that references d.deadline (the step===4 check) and the code
that sets the input min attribute accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9d9148c-ef53-421f-8376-eb2ce194616e
📒 Files selected for processing (3)
frontend/src/App.tsxfrontend/src/__tests__/create-bounty-wizard.test.tsxfrontend/src/pages/CreateBountyPage.tsx
| <Route path="/leaderboard" element={<LeaderboardPage />} /> | ||
| <Route path="/bounties/create" element={<CreateBountyPage />} /> | ||
| <Route path="*" element={<Navigate to="/leaderboard" replace />} /> |
There was a problem hiding this comment.
/bounties is still an unmapped destination.
CreateBountyPage navigates to /bounties on Line 55 and Line 57, but this router only registers /leaderboard and /bounties/create. Both paths will fall through line 20 and redirect to /leaderboard instead of the bounty board.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/App.tsx` around lines 18 - 20, Router is missing a route for
"/bounties" so navigations from CreateBountyPage to "/bounties" get redirected
to "/leaderboard"; add a Route for path="/bounties" that renders the proper
component (e.g., BountiesPage or the bounty board component) alongside the
existing Route entries, update the route list that includes Route
path="/bounties/create" and Route path="*" so the new Route for "/bounties"
appears before the catch-all Navigate, and ensure the component name matches
what CreateBountyPage expects when it redirects to "/bounties".
| export const TIER_INFO: Record<WizardTier,{label:string;rules:string}> = { | ||
| 1:{label:'T1 - Quick Fix',rules:'$50-$500. Bug fixes, docs.'}, | ||
| 2:{label:'T2 - Standard',rules:'$500-$5k. Features, integrations.'}, | ||
| 3:{label:'T3 - Major',rules:'$5k-$50k. Audits, core modules.'}, |
There was a problem hiding this comment.
The reward contract does not match Issue #24's tier rules.
The wizard advertises dollar ranges in TIER_INFO, accepts any reward >= 0.01, and labels the preview as USDC. The linked issue/PR objective requires tier-specific $FNDRY rules (including the T2 target parameters), so the current client can publish rewards that violate the tier contract while showing the wrong token to the user.
Also applies to: 30-30, 64-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/CreateBountyPage.tsx` around lines 10 - 13, TIER_INFO
currently advertises USD ranges and the UI allows any reward >= 0.01 while
previewing "USDC", but the contract requires tier-specific $FNDRY rules
(including the T2 parameters from Issue `#24`); update TIER_INFO to reflect the
correct FNDRY ranges/labels for WizardTier 1–3, then add enforcement in the
bounty creation validation (the reward input validation in CreateBountyPage
and/or its onSubmit/validateReward handler) to reject values outside the
tier-specific min/max and to require the FNDRY token type, and update the
preview rendering (the reward preview component or JSX in CreateBountyPage that
currently shows "USDC") to display "FNDRY" and the validated FNDRY amount.
Ensure messages/labels use the same tier names from TIER_INFO so UI and
validation are consistent.
| const b={title:draft.title.trim(),description:draft.description+(draft.requirements.length?'\n\n## Requirements\n'+draft.requirements.map(r=>`- ${r}`).join('\n'):''),tier:draft.tier,reward_amount:Number(draft.reward_amount),required_skills:draft.required_skills.map(s=>s.toLowerCase()),deadline:draft.deadline||null,created_by:addr}; | ||
| try{const r=await fetch('/api/bounties',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify(b)});if(!r.ok)throw new Error(`Server error: ${r.status}`);localStorage.removeItem(dk(addr));nav('/bounties');}catch(e){setErr(e instanceof Error?e.message:'Failed.');}finally{setBusy(false);} |
There was a problem hiding this comment.
Selected category is dropped before publish.
Step 4 makes draft.category mandatory, but the request body assembled here never includes it and the preview never shows it. Every bounty created through this flow will be uncategorized even though the user was forced to choose a category.
Minimal fix
- const b={title:draft.title.trim(),description:draft.description+(draft.requirements.length?'\n\n## Requirements\n'+draft.requirements.map(r=>`- ${r}`).join('\n'):''),tier:draft.tier,reward_amount:Number(draft.reward_amount),required_skills:draft.required_skills.map(s=>s.toLowerCase()),deadline:draft.deadline||null,created_by:addr};
+ const b={title:draft.title.trim(),description:draft.description+(draft.requirements.length?'\n\n## Requirements\n'+draft.requirements.map(r=>`- ${r}`).join('\n'):''),tier:draft.tier,category:draft.category,reward_amount:Number(draft.reward_amount),required_skills:draft.required_skills.map(s=>s.toLowerCase()),deadline:draft.deadline||null,created_by:addr};-<div>Skills: {D.required_skills.join(', ')||'-'}</div>
+<div>Category: {D.category || '-'}</div>
+<div>Skills: {D.required_skills.join(', ')||'-'}</div>Also applies to: 63-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/CreateBountyPage.tsx` around lines 54 - 55, The request
body object b is missing the selected category so draft.category is dropped;
update the object construction in CreateBountyPage (variable b) to include
category: draft.category (and add the same property to the equivalent POST
payloads at the other occurrence around lines 63-65) so the API and preview
receive and persist the chosen category.
7-step auth-gated wizard with wallet-scoped draft persistence, XSS-sanitized markdown preview, and server-derived ownership. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ItachiDevv
force-pushed
the
fix/issue-24-bounty-creation-wizard
branch
from
775e82a to
3708418
Compare
|
Issues found:
Suggestions:
Please address these items and push an update. SolFoundry Review Bot |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
frontend/src/pages/CreateBountyPage.tsx (5)
28-33:⚠️ Potential issue | 🟠 MajorThe description preview is still not a real Markdown preview.
Lines 28-33 only regex-strip parts of raw HTML, and Line 102 injects that string directly. Markdown input like
# Heading,- item, or[link](...)is never rendered, so users still cannot verify formatting before publish. Because the same path relies ondangerouslySetInnerHTML, the XSS guarantee is also riding on an ad hoc sanitizer in a security-sensitive code path. As per coding guidelines,frontend/**: React/TypeScript frontend. Check: XSS prevention.Also applies to: 92-92, 102-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/CreateBountyPage.tsx` around lines 28 - 33, The preview is treating raw Markdown as plain text and using an unsafe ad-hoc sanitizer: replace the regex-based sanitizeHtml used in CreateBountyPage with a pipeline that (1) parses Markdown into HTML using a proper renderer (e.g. marked/markdown-it) and (2) sanitizes the rendered HTML with a vetted library (e.g. DOMPurify) before inserting via dangerouslySetInnerHTML; remove or stop using the regex-based sanitizeHtml function, configure the sanitizer with an allow-list of safe tags/attributes and strip scripts/data: and javascript: URIs, and ensure the preview code path that sets dangerouslySetInnerHTML uses the sanitized output.
45-46:⚠️ Potential issue | 🟠 MajorThe deadline logic is using UTC dates against local user input.
input[type="date"]returns a calendar date, but Line 96 derivesminfromtoISOString()and Line 45 parsesd.deadlinethroughnew Date(...). For a user in America/Los_Angeles on March 19, 2026 after 17:00,toISOString().split('T')[0]already becomes2026-03-20, and selecting2026-03-20later compares as2026-03-19T17:00:00-07:00. Same-day deadlines are therefore rejected or shifted depending on timezone.Also applies to: 96-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/CreateBountyPage.tsx` around lines 45 - 46, The deadline comparison uses full ISO timestamps causing timezone shifts: when evaluating the step check (s) against d.deadline and when computing the date input min from toISOString().split('T')[0]. Treat deadlines as local calendar dates instead of UTC timestamps by normalizing both sides to date-only values — e.g., convert d.deadline to a local YYYY-MM-DD string (or construct a Date at local midnight) before comparing, and compute the input min using the same local-date logic rather than toISOString().split('T')[0]; update the check around s===5 and the min calculation so both use the same date-only representation to avoid timezone-induced off-by-one errors.
12-15:⚠️ Potential issue | 🟠 MajorReward validation does not enforce the FNDRY tier contract.
Line 12 defaults the draft to
USDC, Line 14 only allowsUSDC/SOL, and Lines 44-45 accept any positive number regardless of tier. Issue#24requires tier-specific$FNDRYrules, including the T2 500,000 FNDRY target, so this wizard currently accepts and previews rewards that violate the bounty contract.Also applies to: 39-46, 95-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/CreateBountyPage.tsx` around lines 12 - 15, The currency list and reward validation need FNDRY/tier-specific rules: add 'FNDRY' to CUR (replace CUR = ['USDC','SOL'] with including 'FNDRY'), then update the reward validation logic that reads BountyDraft.rewardAmount/BountyDraft.currency and tier (used in preview/submit handlers and any validateReward function) to enforce numeric >0 for non-FNDRY as before, but for currency === 'FNDRY' enforce the contract rules (e.g., if draft.tier === 'T2' require Number(rewardAmount) >= 500000); also adjust any preview/summary code to display FNDRY amounts and validation errors consistently. Ensure the checks are applied where reward validation currently occurs (functions handling submit/preview and any validateReward/formatReward utilities) and surface clear validation messages when the tier/currency constraints are violated.
8-15:⚠️ Potential issue | 🟠 MajorThe wizard contract is still missing required Issue
#24fields.Issue
#24requires a tier rule preview, a requirements checklist builder, and a category + skills step. HereSTEPSis onlyTitle/Description/Tier/Skills/Reward/Deadline/Review,BountyDraft/BountySubmitPayloadhave norequirementsorcategory, and the review block on Lines 99-100 cannot surface either field. This page cannot persist or submit an issue-compliant bounty.Also applies to: 35-37, 90-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/CreateBountyPage.tsx` around lines 8 - 15, STEPS and the draft/submit types are missing the Issue `#24` fields — add a "Category" and "Requirements" step to STEPS (e.g., include 'Category' and 'Requirements'), extend BountyDraft and BountySubmitPayload to include category: string and requirements: Array<{id: string; text: string; required: boolean}> (or similar checklist item shape), update EMPTY to provide defaults for category and requirements, and ensure the Tier step (where TIERS is used) renders a tier rule preview UI and that the Review rendering logic (the review block) surfaces the category, the rendered tier rule preview, and the full requirements checklist so they are persisted and submitted; update any payload conversion in submit code to serialize requirements correctly and keep rewardAmount typing consistent between BountyDraft (string) and BountySubmitPayload (number).
55-56:⚠️ Potential issue | 🟠 MajorThe page is still gated only on wallet connectivity.
CreateBountyPageonly readsconnectedfromuseWalletConnection()on Lines 55-56, and the render guard on Lines 84-85 shows the full wizard whenever that flag is true. The hook implementation infrontend/src/hooks/useWallet.tsexposes wallet state only, so the GitHub-auth half of Issue#24is still unenforced in the client.Also applies to: 84-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/CreateBountyPage.tsx` around lines 55 - 56, CreateBountyPage currently only checks the wallet state (connected/displayInfo/wa) from useWalletConnection and renders the wizard whenever connected is true; update the component to also read the GitHub auth state (e.g., from the existing GitHub auth hook — import and call useGithubAuth or the equivalent hook that exposes githubAuthenticated/githubUser) and change the render guard that shows the wizard (currently gated by connected) to require both connected && githubAuthenticated (or githubUser != null). Ensure any UI branches that prompt for authentication now guide the user to both connect their wallet and authenticate with GitHub before showing the full wizard.frontend/src/__tests__/create-bounty-wizard.test.tsx (1)
27-44: 🛠️ Refactor suggestion | 🟠 MajorThe flow test still stops before the preview/publish contract is exercised.
This case only checks that
description-previewandbtn-submitexist. It never asserts what the preview renders, never clicks submit, and never inspects the/api/bountiesrequest body or draft cleanup. A regression in thedangerouslySetInnerHTMLpath infrontend/src/pages/CreateBountyPage.tsxLine 102 or the publish call on Lines 72-75 would still pass here. As per coding guidelines,frontend/**: React/TypeScript frontend. Check: XSS prevention; Integration with existing components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/__tests__/create-bounty-wizard.test.tsx` around lines 27 - 44, Extend the test to fully exercise the preview and publish flow: after reaching the review step (in create-bounty-wizard.test.tsx) assert the actual rendered preview content from the element with testid 'description-preview' (e.g., that markdown/HTML is sanitized and does not render raw script tags to cover the dangerouslySetInnerHTML path in CreateBountyPage.tsx), then mock/intercept the network request for the publish action (POST to /api/bounties) invoked by the btn-submit click, click the 'btn-submit' button, await the request and assert the request body contains the expected fields (title, description, reward, deadline, tags) and that no unsafe HTML was sent, and finally assert draft cleanup (e.g., localStorage key removed or expected cleanup API call) after publish; reference elements 'review-step', 'description-preview', 'btn-submit' and the CreateBountyPage.tsx publish/dangerouslySetInnerHTML code paths when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/__tests__/create-bounty-wizard.test.tsx`:
- Around line 83-87: Add a test that verifies isolation when reconnecting as a
different wallet: create a draft for wallet A (use existing helpers C(), R() to
mount/save), then simulate reconnecting as wallet B (change the wallet
identifier used by the test to a different value, e.g. B, and call C() and R()
again) and assert that localStorage.getItem(`solfoundry_bounty_draft_${B}`) is
null while the original key `solfoundry_bounty_draft_${A}` still exists and was
not copied; update or add a test named like "does not leak draft between
wallets" using the same helpers (C(), R()) and variables (A, B) to locate the
logic to change.
In `@frontend/src/pages/CreateBountyPage.tsx`:
- Around line 72-75: The submit() handler is re-entrant: add an in-flight guard
using a new state (e.g., submitting) so submit() returns early if submitting is
true, setSubmitting(true) before the async POST and setSubmitting(false) in a
finally block (or setDone(true) and leave submitting false as appropriate), and
update the "Submit Bounty" button component to be disabled when submitting (or
when done) so users cannot double-click; reference the submit() function, the
setDone/setErr state setters, and the button rendering for the "Submit Bounty"
action when making these changes.
- Around line 62-67: The wallet-scoped draft useEffect doesn't clear the
in-memory draft when the active wallet (wa) changes, causing drafts to be
written under the wrong wallet key; update the first useEffect (the one reading
localStorage using dk(wa) and validateDraftShape) to explicitly reset the
in-memory draft (setDraft(undefined) or initial value) whenever wa is falsy,
when no saved draft is found (!r), or in the catch block, so stale draft data
isn't carried over and later written by the second useEffect; keep
validateDraftShape, dk, draft, wa, and done logic intact but ensure the
read-effect always clears draft on wallet-change/no-data/error.
---
Duplicate comments:
In `@frontend/src/__tests__/create-bounty-wizard.test.tsx`:
- Around line 27-44: Extend the test to fully exercise the preview and publish
flow: after reaching the review step (in create-bounty-wizard.test.tsx) assert
the actual rendered preview content from the element with testid
'description-preview' (e.g., that markdown/HTML is sanitized and does not render
raw script tags to cover the dangerouslySetInnerHTML path in
CreateBountyPage.tsx), then mock/intercept the network request for the publish
action (POST to /api/bounties) invoked by the btn-submit click, click the
'btn-submit' button, await the request and assert the request body contains the
expected fields (title, description, reward, deadline, tags) and that no unsafe
HTML was sent, and finally assert draft cleanup (e.g., localStorage key removed
or expected cleanup API call) after publish; reference elements 'review-step',
'description-preview', 'btn-submit' and the CreateBountyPage.tsx
publish/dangerouslySetInnerHTML code paths when making these changes.
In `@frontend/src/pages/CreateBountyPage.tsx`:
- Around line 28-33: The preview is treating raw Markdown as plain text and
using an unsafe ad-hoc sanitizer: replace the regex-based sanitizeHtml used in
CreateBountyPage with a pipeline that (1) parses Markdown into HTML using a
proper renderer (e.g. marked/markdown-it) and (2) sanitizes the rendered HTML
with a vetted library (e.g. DOMPurify) before inserting via
dangerouslySetInnerHTML; remove or stop using the regex-based sanitizeHtml
function, configure the sanitizer with an allow-list of safe tags/attributes and
strip scripts/data: and javascript: URIs, and ensure the preview code path that
sets dangerouslySetInnerHTML uses the sanitized output.
- Around line 45-46: The deadline comparison uses full ISO timestamps causing
timezone shifts: when evaluating the step check (s) against d.deadline and when
computing the date input min from toISOString().split('T')[0]. Treat deadlines
as local calendar dates instead of UTC timestamps by normalizing both sides to
date-only values — e.g., convert d.deadline to a local YYYY-MM-DD string (or
construct a Date at local midnight) before comparing, and compute the input min
using the same local-date logic rather than toISOString().split('T')[0]; update
the check around s===5 and the min calculation so both use the same date-only
representation to avoid timezone-induced off-by-one errors.
- Around line 12-15: The currency list and reward validation need
FNDRY/tier-specific rules: add 'FNDRY' to CUR (replace CUR = ['USDC','SOL'] with
including 'FNDRY'), then update the reward validation logic that reads
BountyDraft.rewardAmount/BountyDraft.currency and tier (used in preview/submit
handlers and any validateReward function) to enforce numeric >0 for non-FNDRY as
before, but for currency === 'FNDRY' enforce the contract rules (e.g., if
draft.tier === 'T2' require Number(rewardAmount) >= 500000); also adjust any
preview/summary code to display FNDRY amounts and validation errors
consistently. Ensure the checks are applied where reward validation currently
occurs (functions handling submit/preview and any validateReward/formatReward
utilities) and surface clear validation messages when the tier/currency
constraints are violated.
- Around line 8-15: STEPS and the draft/submit types are missing the Issue `#24`
fields — add a "Category" and "Requirements" step to STEPS (e.g., include
'Category' and 'Requirements'), extend BountyDraft and BountySubmitPayload to
include category: string and requirements: Array<{id: string; text: string;
required: boolean}> (or similar checklist item shape), update EMPTY to provide
defaults for category and requirements, and ensure the Tier step (where TIERS is
used) renders a tier rule preview UI and that the Review rendering logic (the
review block) surfaces the category, the rendered tier rule preview, and the
full requirements checklist so they are persisted and submitted; update any
payload conversion in submit code to serialize requirements correctly and keep
rewardAmount typing consistent between BountyDraft (string) and
BountySubmitPayload (number).
- Around line 55-56: CreateBountyPage currently only checks the wallet state
(connected/displayInfo/wa) from useWalletConnection and renders the wizard
whenever connected is true; update the component to also read the GitHub auth
state (e.g., from the existing GitHub auth hook — import and call useGithubAuth
or the equivalent hook that exposes githubAuthenticated/githubUser) and change
the render guard that shows the wizard (currently gated by connected) to require
both connected && githubAuthenticated (or githubUser != null). Ensure any UI
branches that prompt for authentication now guide the user to both connect their
wallet and authenticate with GitHub before showing the full wizard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd332a55-63f6-4599-9c22-f77e72d2a646
📒 Files selected for processing (3)
frontend/src/App.tsxfrontend/src/__tests__/create-bounty-wizard.test.tsxfrontend/src/pages/CreateBountyPage.tsx
| describe('Draft localStorage', () => { | ||
| beforeEach(() => localStorage.clear()); | ||
| it('scopes to wallet address', () => { C(); R(); expect(localStorage.getItem(`solfoundry_bounty_draft_${A}`)).not.toBeNull(); }); | ||
| it('recovers from corrupt data', () => { localStorage.setItem(`solfoundry_bounty_draft_${A}`, '{"title":123}'); C(); R(); expect(JSON.parse(localStorage.getItem(`solfoundry_bounty_draft_${A}`)!).title).toBe(''); }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
The wallet-scoping tests do not cover reconnecting as a different wallet.
scopes to wallet address only proves that one key is written for A. It would still pass if a draft loaded for wallet A were copied into wallet B during a reconnect, which is exactly the isolation contract this page is supposed to guarantee.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/__tests__/create-bounty-wizard.test.tsx` around lines 83 - 87,
Add a test that verifies isolation when reconnecting as a different wallet:
create a draft for wallet A (use existing helpers C(), R() to mount/save), then
simulate reconnecting as wallet B (change the wallet identifier used by the test
to a different value, e.g. B, and call C() and R() again) and assert that
localStorage.getItem(`solfoundry_bounty_draft_${B}`) is null while the original
key `solfoundry_bounty_draft_${A}` still exists and was not copied; update or
add a test named like "does not leak draft between wallets" using the same
helpers (C(), R()) and variables (A, B) to locate the logic to change.
| useEffect(() => { | ||
| if (!wa) return; | ||
| try { const r = localStorage.getItem(dk(wa)); if (!r) return; const v = validateDraftShape(JSON.parse(r)); if (v) setDraft(v); else localStorage.removeItem(dk(wa)); } | ||
| catch { localStorage.removeItem(dk(wa)); } | ||
| }, [wa]); | ||
| useEffect(() => { if (!wa||done) return; try { localStorage.setItem(dk(wa), JSON.stringify(draft)); } catch {} }, [draft, wa, done]); |
There was a problem hiding this comment.
Wallet-scoped drafts leak when the active wallet changes.
If wallet A has a draft in memory and the user connects wallet B with no saved draft, Line 64 returns without resetting draft. Line 67 then writes wallet A's stale draft under wallet B's key, which breaks per-wallet isolation and can surface one account's unpublished bounty details in another account.
Reset the in-memory draft when the wallet changes
useEffect(() => {
- if (!wa) return;
- try { const r = localStorage.getItem(dk(wa)); if (!r) return; const v = validateDraftShape(JSON.parse(r)); if (v) setDraft(v); else localStorage.removeItem(dk(wa)); }
- catch { localStorage.removeItem(dk(wa)); }
+ if (!wa) {
+ setDraft(EMPTY);
+ return;
+ }
+ try {
+ const r = localStorage.getItem(dk(wa));
+ if (!r) {
+ setDraft(EMPTY);
+ return;
+ }
+ const v = validateDraftShape(JSON.parse(r));
+ if (v) setDraft(v);
+ else {
+ localStorage.removeItem(dk(wa));
+ setDraft(EMPTY);
+ }
+ } catch {
+ localStorage.removeItem(dk(wa));
+ setDraft(EMPTY);
+ }
}, [wa]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/CreateBountyPage.tsx` around lines 62 - 67, The
wallet-scoped draft useEffect doesn't clear the in-memory draft when the active
wallet (wa) changes, causing drafts to be written under the wrong wallet key;
update the first useEffect (the one reading localStorage using dk(wa) and
validateDraftShape) to explicitly reset the in-memory draft (setDraft(undefined)
or initial value) whenever wa is falsy, when no saved draft is found (!r), or in
the catch block, so stale draft data isn't carried over and later written by the
second useEffect; keep validateDraftShape, dk, draft, wa, and done logic intact
but ensure the read-effect always clears draft on wallet-change/no-data/error.
| const submit = useCallback(async () => { | ||
| setErr(null); | ||
| try { const r = await fetch('/api/bounties', { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', body: JSON.stringify(buildPayload(draft)) }); if (!r.ok) throw new Error(`Server error: ${r.status}`); setDone(true); if (wa) localStorage.removeItem(dk(wa)); } | ||
| catch (e) { setErr(e instanceof Error ? e.message : 'Submission failed'); } |
There was a problem hiding this comment.
The publish request is re-entrant.
There is no in-flight guard in submit(), and the button on Line 109 stays enabled while the POST is pending. Double-clicking Submit Bounty will send multiple /api/bounties requests, which is a non-idempotent write path and can create duplicate issues or duplicate funding actions.
Block duplicate submissions while the request is in flight
export function CreateBountyPage() {
const [col, setCol] = useState(false);
const { connected, displayInfo } = useWalletConnection();
const wa = displayInfo?.address ?? '';
const [step, setStep] = useState(0);
const [draft, setDraft] = useState<BountyDraft>(EMPTY);
const [done, setDone] = useState(false);
+ const [submitting, setSubmitting] = useState(false);
const [err, setErr] = useState<string|null>(null);
@@
const submit = useCallback(async () => {
+ if (submitting) return;
setErr(null);
- try { const r = await fetch('/api/bounties', { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', body: JSON.stringify(buildPayload(draft)) }); if (!r.ok) throw new Error(`Server error: ${r.status}`); setDone(true); if (wa) localStorage.removeItem(dk(wa)); }
+ setSubmitting(true);
+ try { const r = await fetch('/api/bounties', { method: 'POST', headers: { 'Content-Type': 'application/json' }, credentials: 'include', body: JSON.stringify(buildPayload(draft)) }); if (!r.ok) throw new Error(`Server error: ${r.status}`); setDone(true); if (wa) localStorage.removeItem(dk(wa)); }
catch (e) { setErr(e instanceof Error ? e.message : 'Submission failed'); }
- }, [draft, wa]);
+ finally { setSubmitting(false); }
+ }, [draft, wa, submitting]);
@@
- : <button type="button" onClick={submit} data-testid="btn-submit" className={BC+' bg-[`#00FF88`] text-black'}>Submit Bounty</button>}
+ : <button type="button" onClick={submit} disabled={submitting} data-testid="btn-submit" className={BC+' bg-[`#00FF88`] text-black disabled:opacity-40'}>Submit Bounty</button>}Also applies to: 109-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/CreateBountyPage.tsx` around lines 72 - 75, The submit()
handler is re-entrant: add an in-flight guard using a new state (e.g.,
submitting) so submit() returns early if submitting is true, setSubmitting(true)
before the async POST and setSubmitting(false) in a finally block (or
setDone(true) and leave submitting false as appropriate), and update the "Submit
Bounty" button component to be disabled when submitting (or when done) so users
cannot double-click; reference the submit() function, the setDone/setErr state
setters, and the button rendering for the "Submit Bounty" action when making
these changes.
1 participant
Description
Multi-step bounty creation wizard with 7 steps, validation, localStorage draft saving, and backend integration.
Closes #24
Steps
Features
Solana Wallet for Payout
Wallet:
97VihHW2Br7BKUU16c7RxjiEMHsD4dWisGDT2Y3LyJxFChecklist
Summary by CodeRabbit