Feature/maintainer analytics - #605
Conversation
|
@sweetesty is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds a maintainer analytics dashboard with route navigation and charts, updates backend response schemas and routes, improves frontend accessibility and copy UI, refines recommendation scoring, extends Vite build config for PWA and CSP, adds Soroban contract CI, and modernizes GitHub Actions workflows and package dependencies. ChangesMaintainer Analytics Dashboard Feature
Backend Schemas, Routing, and Service Updates
Frontend UX, Accessibility, and Algorithm Updates
Frontend Build Tooling and Configuration
GitHub Actions, PR Templates, and Repository Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
|
@sweetesty 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! 🚀 |
| @@ -1,6 +1,10 @@ | |||
|
|
|||
|
|
|||
| import { ReactNode, useState, useCallback, useEffect, useRef } from "react"; | |||
|
|
||
|
|
||
| import { ReactNode, useState, useCallback, useEffect, useRef } from "react"; | ||
| import { ArrowUpRight, Check, Clock, Copy, Share2, Printer } from "lucide-react"; |
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/validation/schemas.ts (1)
77-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix bounty
amountvalidation to avoid IEEE-754 rounding mismatches
backend/src/validation/schemas.ts(createBountySchema.amount, lines 77-84) usesNumber.isInteger(value * 10_000_000), which can reject valid “<= 7 decimal places” inputs (e.g.,10.000001) thatvalidateBountyAmount()inbackend/src/app.tsaccepts (it checksamount.toString().split('.')[1].length <= 7). SincecreateBountySchema.safeParse()runs beforevalidateBountyAmount(), this blocks valid/api/bountiesrequests.Update the schema to validate decimal places without float-math multiplication (e.g., mirror
validateBountyAmount()’s decimal-length logic or validate via string/regex before coercion).🤖 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 `@backend/src/validation/schemas.ts` around lines 77 - 84, The current refine in createBountySchema.amount multiplies a float (Number.isInteger(value * 10_000_000)) and can fail due to IEEE-754 rounding; change the schema to validate decimal places using the original string representation (or a regex) instead of float multiplication—for example, use z.preprocess to capture the incoming value as a string and then refine by checking String(input).split('.')[1]?.length <= 7 (or match /^\d+(\.\d{1,7})?$/) before coercing to number so the schema agrees with validateBountyAmount().
🟡 Minor comments (7)
docs/FAQ.md-99-103 (1)
99-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix code block language identifier.
Line 102 contains JavaScript code (
localStorage.clear()) but uses abashcode block identifier. This should bejavascriptorjsto provide correct syntax highlighting and avoid confusion.📝 Proposed fix
## Frontend Reset Clear browser storage: -```bash +```javascript localStorage.clear()</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/FAQ.mdaround lines 99 - 103, Change the code fence language label for
the block containing localStorage.clear() from "bash" to a JavaScript label
(e.g., "javascript" or "js"); locate the fenced code block that wraps the token
localStorage.clear() and replace the opening fence identifier so the block
becomes a JavaScript snippet and renders with correct syntax highlighting.</details> </blockquote></details> <details> <summary>frontend/src/ContributorProfilePage.tsx-34-53 (1)</summary><blockquote> `34-53`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Clear stale errors before loading the next contributor.** Lines 34-53 only ever set `error` on failure. After one failed fetch, a later successful load still leaves the old error banner on screen. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@frontend/src/ContributorProfilePage.tsxaround lines 34 - 53, The effect
that loads contributor data (useEffect) only sets error on failures, leaving
stale errors after subsequent successful loads; update the effect that calls
fetchContributorBounties and fetchLeaderboard to clear the existing error at the
start (call setError(null or undefined) before initiating the fetches), so that
successful responses (handled in the then blocks for fetchContributorBounties
and fetchLeaderboard) won’t show an old error banner; reference the useEffect,
setError, fetchContributorBounties, fetchLeaderboard, and the active flag when
making the change.</details> </blockquote></details> <details> <summary>frontend/src/App.tsx-842-845 (1)</summary><blockquote> `842-845`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Make the header logo keyboard-activatable.** Lines 842-845 and 1018-1021 expose a focusable element with `role="link"`, but there is no Enter/Space handler. Keyboard users can tab to it and still cannot navigate home. Also applies to: 1018-1021 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@frontend/src/App.tsxaround lines 842 - 845, The header logo div with
className "nav-logo" uses role="link" and tabIndex but lacks keyboard
activation; add an onKeyDown handler to the elements that listens for Enter and
Space (e.g., key === "Enter" or key === " " / keyCode 13/32) and calls the same
navigate("/") function (with preventDefault on Space) so keyboard users can
activate the link; apply the same change to both occurrences of the nav-logo
element (the ones that render the Rocket and "Stellar Bounty Board") to keep
behavior consistent.</details> </blockquote></details> <details> <summary>frontend/src/MaintainerAnalyticsPage.tsx-33-42 (1)</summary><blockquote> `33-42`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add the no-data empty state for the status chart.** Lines 33-42 can produce an all-zero dataset, but Lines 119-146 always render the `BarChart`. That misses the PR requirement for an empty state when the maintainer has no bounty data and leaves users with a blank chart frame instead. <details> <summary>Proposed fix</summary> ```diff const statusData = useMemo(() => { return [ { name: "Open", bounties: metrics.openCount, fill: "`#1ebd93`" }, { name: "Reserved", bounties: metrics.reservedCount, fill: "`#4b7fc4`" }, { name: "Submitted", bounties: metrics.submittedCount, fill: "`#d9802e`" }, { name: "Released", bounties: metrics.releasedCount, fill: "`#3a8f2a`" }, { name: "Refunded", bounties: metrics.refundedCount, fill: "`#b8554b`" }, { name: "Expired", bounties: metrics.expiredCount, fill: "`#777777`" }, ]; }, [metrics]); + + const hasStatusData = useMemo( + () => statusData.some((item) => item.bounties > 0), + [statusData], + ); @@ <div className="chart-wrapper" style={{ height: 260 }}> - <ResponsiveContainer width="100%" height={260}> - <BarChart - data={statusData} - margin={{ top: 10, right: 10, left: -20, bottom: 0 }} - > - <CartesianGrid strokeDasharray="3 3" vertical={false} stroke="rgba(54,63,59,0.08)" /> - <XAxis dataKey="name" tick={{ fontSize: 10, fill: "var(--muted)" }} /> - <YAxis allowDecimals={false} tick={{ fontSize: 10, fill: "var(--muted)" }} /> - <Tooltip - contentStyle={{ - background: "var(--ink)", - color: "`#fff`", - borderRadius: "8px", - border: "none", - fontSize: "12px", - }} - /> - <Bar dataKey="bounties" fill="`#4b7fc4`" radius={[4, 4, 0, 0]} /> - </BarChart> - </ResponsiveContainer> + {!hasStatusData ? ( + <div className="empty-state" style={{ height: "100%", justifyContent: "center" }}> + No bounty status data available yet. + </div> + ) : ( + <ResponsiveContainer width="100%" height={260}> + <BarChart + data={statusData} + margin={{ top: 10, right: 10, left: -20, bottom: 0 }} + > + <CartesianGrid strokeDasharray="3 3" vertical={false} stroke="rgba(54,63,59,0.08)" /> + <XAxis dataKey="name" tick={{ fontSize: 10, fill: "var(--muted)" }} /> + <YAxis allowDecimals={false} tick={{ fontSize: 10, fill: "var(--muted)" }} /> + <Tooltip + contentStyle={{ + background: "var(--ink)", + color: "`#fff`", + borderRadius: "8px", + border: "none", + fontSize: "12px", + }} + /> + <Bar dataKey="bounties" fill="`#4b7fc4`" radius={[4, 4, 0, 0]} /> + </BarChart> + </ResponsiveContainer> + )} </div>Also applies to: 119-146
🤖 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 `@frontend/src/MaintainerAnalyticsPage.tsx` around lines 33 - 42, The status chart can be all zeros; update the logic around the useMemo'd statusData and the BarChart rendering (referencing statusData and the BarChart block currently at lines ~119-146) to detect a no-data case and render an empty state instead of the chart: compute a hasData boolean (e.g., statusData.some(item => item.bounties > 0) or sum of metrics values) and, when hasData is false, return the empty-state UI (placeholder text/graphic) for the maintainer rather than rendering the BarChart; keep the useMemo for statusData but guard the BarChart render with this check.frontend/src/ErrorBoundary.tsx-58-58 (1)
58-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unnecessary type assertion.
The
as anytype assertion suppresses type safety and is unnecessary. React 19 properly handlesReact.ReactNodein render methods without explicit casting.🔧 Proposed fix
- return this.props.children as any; + return this.props.children;🤖 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 `@frontend/src/ErrorBoundary.tsx` at line 58, The render method in ErrorBoundary is using an unnecessary and unsafe type assertion: replace the "return this.props.children as any;" statement in the ErrorBoundary render function with a plain "return this.props.children;" (ensure the render signature returns React.ReactNode if needed) to restore proper typing and remove the redundant cast.frontend/src/ErrorBoundary.test.tsx-25-28 (1)
25-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIncomplete test coverage for retry functionality.
The test clicks "Try again" but includes no assertions to verify the retry behavior. Since
Bombalways throws, the error boundary will immediately catch the error again after retry.To properly test the retry mechanism, either:
- Add assertions to verify the error boundary catches the error again after retry
- Create a separate test with a component that can conditionally recover (e.g., a component that throws only on first render)
🧪 Suggested test enhancement
+ +function ConditionalBomb({ shouldThrow }: { shouldThrow: boolean }) { + if (shouldThrow) throw new Error("boom"); + return <div>Success</div>; +} + +test("successfully recovers when child stops throwing", async () => { + const [shouldThrow, setShouldThrow] = React.useState(true); + + render( + <ErrorBoundary componentName="TestComponent"> + <ConditionalBomb shouldThrow={shouldThrow} /> + </ErrorBoundary> + ); + + expect(screen.getByText(/Something went wrong/i)).toBeInTheDocument(); + + // Fix the underlying issue + setShouldThrow(false); + + // Retry should now succeed + await userEvent.click(screen.getByRole("button", { name: /Try again/i })); + expect(screen.getByText("Success")).toBeInTheDocument(); +});🤖 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 `@frontend/src/ErrorBoundary.test.tsx` around lines 25 - 28, The test currently clicks the "Try again" button (await userEvent.click(button)) but has no assertions to verify retry behavior; update the test to either (A) assert that after clicking the button the ErrorBoundary re-enters its error state (e.g., expect an error message or fallback UI to be present again since Bomb always throws) by asserting the fallback is shown, or (B) create a separate test that mounts a recoverable component (replace Bomb with a component that throws only on first render) and assert that after userEvent.click(button) the component recovers and the normal UI is rendered; locate and modify the test around the existing userEvent.click(button) call and the Bomb usage to implement one of these two approaches.frontend/tsconfig.node.tsbuildinfo-1-1 (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove tracked TypeScript
*.tsbuildinfobuild artifacts
frontend/tsconfig.node.tsbuildinfo(andfrontend/tsconfig.tsbuildinfo) are committed and not ignored by.gitignore(git check-ignore returns “not ignored”), so they’ll keep generating large, compiler-version-specific diffs. Remove them from source control and add an ignore rule (e.g.,*.tsbuildinfo) to prevent future churn.🤖 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 `@frontend/tsconfig.node.tsbuildinfo` at line 1, The repo has committed TypeScript build artifacts (frontend/tsconfig.node.tsbuildinfo and frontend/tsconfig.tsbuildinfo) that should be removed and ignored; remove them from git tracking (e.g., git rm --cached frontend/tsconfig.node.tsbuildinfo frontend/tsconfig.tsbuildinfo), add a rule like *.tsbuildinfo to your .gitignore (or update existing ignore handling), and commit the .gitignore change and the removal so the large, compiler-specific diffs stop appearing.
🧹 Nitpick comments (2)
.github/workflows/soroban-contract-ci.yml (2)
12-12: ⚡ Quick winDisable credential persistence for security.
The
actions/checkoutaction persists Git credentials by default, which can be exploited by malicious code in subsequent steps. For build/test workflows, credentials are typically not needed after checkout.🛡️ Recommended fix
- uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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/soroban-contract-ci.yml at line 12, The checkout step currently uses actions/checkout@v4 and leaves Git credentials persisted; update the checkout invocation to disable credential persistence by adding persist-credentials: false so the GitHub token is not kept for later steps (modify the step that references actions/checkout@v4 to include persist-credentials: false).
12-48: ⚖️ Poor tradeoffPin all actions to commit SHAs for supply-chain security.
Actions are currently pinned to tags (e.g.,
@v4,@v1), which are mutable and can be updated to point to malicious code. Pinning to commit SHAs prevents tag-retargeting attacks.🔒 Recommended SHA pinning
Use full commit SHAs with comments indicating the version:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4 - uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 # v4To find the SHA for a version, visit the action's repository releases page.
Consider using Dependabot to keep action SHAs up to date:
# .github/dependabot.yml version: 2 updates: - package-ecosystem: "github-actions" directory: "/" schedule: interval: "weekly"🤖 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/soroban-contract-ci.yml around lines 12 - 48, Replace mutable action tags with their corresponding immutable commit SHAs for each "uses:" entry (e.g., the occurrences of actions/checkout@v4, actions/cache@v4, and actions-rs/toolchain@v1) so the workflow pins to a specific commit; update every "uses:" line in the shown blocks to use the full commit SHA and optionally append a trailing comment with the human-friendly tag (e.g., "# v4" or "# v1") to document the version. Ensure you update all three cache blocks and the toolchain action shown so none remain tagged (`@vX`), test the workflow after changes, and consider adding Dependabot configuration to keep these SHAs current.
🤖 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/soroban-contract-ci.yml:
- Around line 66-80: Replace the Linux-specific stat usage that computes SIZE
(the line using stat -c%s "$WASM_FILE") with a POSIX-portable approach; read the
WASM_FILE variable and compute size with a portable wc invocation (e.g., use wc
-c < "$WASM_FILE" or similar) so the SIZE calculation in the section that prints
WASM binary size works on macOS and Linux runners; update the SIZE assignment
and any references that echo "$SIZE" accordingly while keeping the existing
WASM_FILE, GZ_SIZE, and GITHUB_STEP_SUMMARY logic intact.
- Around line 42-48: The workflow step currently uses the archived action
identifier "uses: actions-rs/toolchain@v1" and passes fields like "override:
true" and "profile: minimal"; replace that step to use "dtolnay/rust-toolchain"
instead, remove the unsupported "override" input and avoid duplicating
"--profile minimal" since dtolnay/rust-toolchain applies minimal profile
internally, and ensure the "toolchain: stable" and "target:
wasm32-unknown-unknown" (and "components: clippy" if still needed) are passed
according to dtolnay/rust-toolchain's inputs.
In `@backend/src/app.ts`:
- Around line 502-504: The /api/open-issues route calls listOpenIssues() without
handling rejections; wrap the await listOpenIssues() call in a try/catch inside
the async handler (app.get('/api/open-issues', ...)) and on error call the
project's error responder (e.g., sendError(res, err) or next(err) depending on
existing convention) so errors are returned as JSON instead of causing an
unhandled rejection; on success keep setting Cache-Control and res.json({ data
}) as before.
In `@backend/src/metrics.ts`:
- Around line 1-17: Replace the stubs in metrics.ts with the real prom-client
implementations: import Registry, Counter, Histogram and collectDefaultMetrics
from 'prom-client' and re-export them (so Registry.metrics() and
getSingleMetric() behave correctly and Counter.inc/Histogram.observe actually
record values); update the /api/metrics usage in app (the getMetrics() call) to
return real registry.metrics() output and add an environment flag (e.g.,
METRICS_ENABLED) so metrics are only exposed when enabled; finally add
'prom-client' to backend/package.json dependencies.
In `@backend/src/services/cache.ts`:
- Around line 1-7: The local stub class Redis in backend/src/services/cache.ts
must be removed and replaced with the real ioredis client import so RedisCache
actually performs get/set/del; locate the stubbed "class Redis" definition and
delete it, add an import of the project's ioredis client (matching existing
project style) and ensure RedisCache and getCache() instantiate and use that
ioredis client so methods like RedisCache.get, RedisCache.set and RedisCache.del
call the real client's get/set/del instead of the no-op stub.
In `@backend/test/webhookSecretValidation.test.ts`:
- Around line 11-13: The vi.mock("../src/logger", () => ({ logStructured:
vi.fn() })) call is currently inside beforeEach and must be moved to module
scope so Vitest can hoist it; relocate that vi.mock invocation to the top of the
test file (above any imports or describe blocks) to ensure the logger module is
mocked before it's imported/used, keeping the mocked export name logStructured
unchanged.
In `@docs/ARCHITECTURE.md`:
- Around line 232-263: The state machine docs are inconsistent: reconcile the
Mermaid diagram, ASCII diagram, and transition table so they all match; decide
whether the Disputed state and the transition Submitted --> Refunded
(refund_bounty) are valid, then apply the chosen design everywhere—if Disputed
and Submitted→Refunded are valid, add "Disputed" and the Submitted -> Refunded
(refund_bounty) transition to the ASCII state diagram and update the transition
rules table entry that currently forbids refunds for SUBMITTED; otherwise remove
the Disputed state and the Submitted -> Refunded line (refund_bounty) from the
Mermaid diagram and any sequence diagrams so all three representations are
identical.
In `@frontend/src/App.tsx`:
- Around line 835-876: The maintainer analytics view is currently shown for any
/maintainer/:address even when the user is disconnected; update the render guard
so that MaintainerAnalyticsPage (and the nav "Analytics Dashboard" button) only
render when connectedWallet exists and equals maintainerAddress (use
connectedWallet === maintainerAddress), otherwise show a gated state (e.g.,
prompt to connect or navigate("/") ) so unauthorized viewers cannot access the
page; also update the disconnect logic that clears local storage (the logic
around the disconnect handler) to additionally navigate away or clear
maintainerAddress state when the wallet is disconnected so the gated view is
enforced after disconnect.
- Around line 351-370: The effect treating a failed getMaintainerMetrics call as
a permanent "null = loading" is causing endless "Loading metrics..." UI; add a
separate error state (e.g., maintainerMetricsError via useState) and update the
effect using getMaintainerMetrics so that on success you
setMaintainerMetrics(data), setMaintainerMetricsError(null) and
setMaintainerMetricsLoading(false), and on catch you
setMaintainerMetricsError(error or true) and setMaintainerMetricsLoading(false)
(do NOT leave maintainerMetrics null to signal loading). Then update the render
logic that currently shows "Loading metrics..." when maintainerMetrics is null
to instead show loading only when maintainerMetricsLoading is true and show an
error/ retry UI when maintainerMetricsError is set; reference
getMaintainerMetrics, setMaintainerMetrics, setMaintainerMetricsLoading, and
maintainerMetrics state when making these changes.
In `@frontend/src/ContributorProfilePage.tsx`:
- Around line 73-81: The stats calculation collapses all released bounties into
a single numeric total (totalEarned) and the UI labels it as "XLM", which is
wrong for multi-asset payouts; change the calc in the useMemo (stats) to
aggregate released amounts by asset (e.g., produce totalEarned as a map/object
like { [asset]: sum }) by summing b.amount grouped by b.asset (fallback to a
default asset if missing), keep released as-is, and update any UI that reads
totalEarned (and the component that currently renders the "XLM" label) to
iterate over the asset map and render each asset-specific total instead of
assuming XLM.
- Around line 55-71: The metadata updater in the useEffect
(ContributorProfilePage -> setMeta) incorrectly creates all tags as
meta[name="..."] (breaking Open Graph which requires property="...") and never
restores previous title/meta on unmount, causing leaks across SPA routes; change
setMeta to accept whether the key is an OG/property tag (detect keys starting
with "og:" or "twitter:" as needed) and query/create the element using
property="..." for og: keys and name="..." for others, store the previous
document.title and previous meta content (or whether the tag was newly created)
when mounting, update document.title via document.title = title, and in the
useEffect cleanup restore the saved title and either restore previous meta
content or remove any meta elements you created (use the unique symbol names:
useEffect, setMeta, shortAddress, document.title) so contributor-specific
metadata doesn't leak after unmount.
In `@frontend/src/logger.ts`:
- Around line 1-13: The frontend imports pino in logger.ts (const logger =
pino(...)) and exposes logError/logger, but the frontend package.json lacks
pino; add pino to the frontend's dependencies (or replace the import with a
browser-friendly logger) so builds don't break: either install "pino" into the
frontend package.json dependencies and keep logger.ts as-is, or swap the
implementation to use console (or import "pino-browser"/an equivalent) and
update the import in logger.ts (logError and logger) so the bundler/runtime use
a browser-compatible logger.
In `@frontend/src/recommendations.ts`:
- Around line 56-72: The current matching loop in recommendations.ts (using
normalizedSkills, expandedTokens, and matchCount) must stop using substring
checks (token.includes(skill) || skill.includes(token)) because short skills
like "go" or "ai" match unrelated words; replace that logic so you only count
matches when a normalized skill exactly equals a normalized token (compare exact
terms), or if you need a fallback keep it guarded: only allow substring fallback
when both the skill and token lengths are >= 3 (or another chosen threshold) and
both sides are normalized/boundary-tokenized; update the loop that increments
matchCount to use exact equality (and the optional guarded fallback) instead of
unguarded includes.
- Around line 35-38: Replace the unsafe cast and tighten matching: stop using
(bounty as unknown as Record<string, unknown>).tags and reference the typed
property bounty.tags directly (Bounty.tags?: string[]) in
frontend/src/recommendations.ts; then revise the scoreMatch function so it does
exact token equality or normalized word-boundary matching instead of the broad
substring checks (replace token.includes(skill) || skill.includes(token) with an
exact match or regex/word-split comparison after lowercasing and trimming) to
avoid over-matching and inflate scores.
In `@frontend/vite.config.js`:
- Around line 19-37: The plugin currently injects a
Content-Security-Policy-Report-Only meta tag from cspPlugin.transformIndexHtml
(variable reportOnlyMeta) which browsers ignore; remove the Report-Only meta
injection there and either inject an enforcing Content-Security-Policy meta (use
"Content-Security-Policy" instead of "Content-Security-Policy-Report-Only") or
better yet stop meta injection altogether and configure your hosting/app layer
or Vite dev server to send the "Content-Security-Policy-Report-Only" HTTP
response header (e.g., add a middleware or server.headers config) so report-only
rollout is done via the HTTP header while any in-HTML meta remains enforcing if
needed.
In `@frontend/vite.config.ts`:
- Around line 4-10: Remove the stray token "ain" (it's breaking TS parsing) and
add the missing imports: import { defineConfig } from "vite" and import {
VitePWA } from "vite-plugin-pwa"; keep the existing react import (import react
from "`@vitejs/plugin-react`") and ensure defineConfig(...) and VitePWA(...) are
referenced correctly inside the default export.
---
Outside diff comments:
In `@backend/src/validation/schemas.ts`:
- Around line 77-84: The current refine in createBountySchema.amount multiplies
a float (Number.isInteger(value * 10_000_000)) and can fail due to IEEE-754
rounding; change the schema to validate decimal places using the original string
representation (or a regex) instead of float multiplication—for example, use
z.preprocess to capture the incoming value as a string and then refine by
checking String(input).split('.')[1]?.length <= 7 (or match /^\d+(\.\d{1,7})?$/)
before coercing to number so the schema agrees with validateBountyAmount().
---
Minor comments:
In `@docs/FAQ.md`:
- Around line 99-103: Change the code fence language label for the block
containing localStorage.clear() from "bash" to a JavaScript label (e.g.,
"javascript" or "js"); locate the fenced code block that wraps the token
localStorage.clear() and replace the opening fence identifier so the block
becomes a JavaScript snippet and renders with correct syntax highlighting.
In `@frontend/src/App.tsx`:
- Around line 842-845: The header logo div with className "nav-logo" uses
role="link" and tabIndex but lacks keyboard activation; add an onKeyDown handler
to the elements that listens for Enter and Space (e.g., key === "Enter" or key
=== " " / keyCode 13/32) and calls the same navigate("/") function (with
preventDefault on Space) so keyboard users can activate the link; apply the same
change to both occurrences of the nav-logo element (the ones that render the
Rocket and "Stellar Bounty Board") to keep behavior consistent.
In `@frontend/src/ContributorProfilePage.tsx`:
- Around line 34-53: The effect that loads contributor data (useEffect) only
sets error on failures, leaving stale errors after subsequent successful loads;
update the effect that calls fetchContributorBounties and fetchLeaderboard to
clear the existing error at the start (call setError(null or undefined) before
initiating the fetches), so that successful responses (handled in the then
blocks for fetchContributorBounties and fetchLeaderboard) won’t show an old
error banner; reference the useEffect, setError, fetchContributorBounties,
fetchLeaderboard, and the active flag when making the change.
In `@frontend/src/ErrorBoundary.test.tsx`:
- Around line 25-28: The test currently clicks the "Try again" button (await
userEvent.click(button)) but has no assertions to verify retry behavior; update
the test to either (A) assert that after clicking the button the ErrorBoundary
re-enters its error state (e.g., expect an error message or fallback UI to be
present again since Bomb always throws) by asserting the fallback is shown, or
(B) create a separate test that mounts a recoverable component (replace Bomb
with a component that throws only on first render) and assert that after
userEvent.click(button) the component recovers and the normal UI is rendered;
locate and modify the test around the existing userEvent.click(button) call and
the Bomb usage to implement one of these two approaches.
In `@frontend/src/ErrorBoundary.tsx`:
- Line 58: The render method in ErrorBoundary is using an unnecessary and unsafe
type assertion: replace the "return this.props.children as any;" statement in
the ErrorBoundary render function with a plain "return this.props.children;"
(ensure the render signature returns React.ReactNode if needed) to restore
proper typing and remove the redundant cast.
In `@frontend/src/MaintainerAnalyticsPage.tsx`:
- Around line 33-42: The status chart can be all zeros; update the logic around
the useMemo'd statusData and the BarChart rendering (referencing statusData and
the BarChart block currently at lines ~119-146) to detect a no-data case and
render an empty state instead of the chart: compute a hasData boolean (e.g.,
statusData.some(item => item.bounties > 0) or sum of metrics values) and, when
hasData is false, return the empty-state UI (placeholder text/graphic) for the
maintainer rather than rendering the BarChart; keep the useMemo for statusData
but guard the BarChart render with this check.
In `@frontend/tsconfig.node.tsbuildinfo`:
- Line 1: The repo has committed TypeScript build artifacts
(frontend/tsconfig.node.tsbuildinfo and frontend/tsconfig.tsbuildinfo) that
should be removed and ignored; remove them from git tracking (e.g., git rm
--cached frontend/tsconfig.node.tsbuildinfo frontend/tsconfig.tsbuildinfo), add
a rule like *.tsbuildinfo to your .gitignore (or update existing ignore
handling), and commit the .gitignore change and the removal so the large,
compiler-specific diffs stop appearing.
---
Nitpick comments:
In @.github/workflows/soroban-contract-ci.yml:
- Line 12: The checkout step currently uses actions/checkout@v4 and leaves Git
credentials persisted; update the checkout invocation to disable credential
persistence by adding persist-credentials: false so the GitHub token is not kept
for later steps (modify the step that references actions/checkout@v4 to include
persist-credentials: false).
- Around line 12-48: Replace mutable action tags with their corresponding
immutable commit SHAs for each "uses:" entry (e.g., the occurrences of
actions/checkout@v4, actions/cache@v4, and actions-rs/toolchain@v1) so the
workflow pins to a specific commit; update every "uses:" line in the shown
blocks to use the full commit SHA and optionally append a trailing comment with
the human-friendly tag (e.g., "# v4" or "# v1") to document the version. Ensure
you update all three cache blocks and the toolchain action shown so none remain
tagged (`@vX`), test the workflow after changes, and consider adding Dependabot
configuration to keep these SHAs current.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85a6a50a-b44e-4731-b212-204c399dc044
⛔ Files ignored due to path filters (3)
backend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (50)
.env.example.github/PULL_REQUEST_TEMPLATE.md.github/pull_request_template.md.github/workflows/soroban-contract-ci.ymlCODE_EXAMPLES.mdCONTRIBUTING.mdIMPLEMENTATION_SUMMARY.mdQUICK_START.mdREADME.mdWEBHOOK_SECRET_VALIDATION.mdWEBHOOK_SECURITY_GUIDE.mdbackend/data/bounties.audit.jsonbackend/data/bounties.jsonbackend/package.jsonbackend/src/app.tsbackend/src/index.tsbackend/src/metrics.tsbackend/src/middleware/auth.tsbackend/src/services/cache.tsbackend/src/validation/schemas.tsbackend/src/validation/webhookSecretValidation.tsbackend/test/authMiddleware.test.tsbackend/test/openapi.contract.test.tsbackend/test/webhookSecretValidation.test.tsdocs/ARCHITECTURE.mddocs/FAQ.mdfrontend/package.jsonfrontend/src/App.tsxfrontend/src/BountyDetailPage.tsxfrontend/src/ContributorProfilePage.test.tsxfrontend/src/ContributorProfilePage.tsxfrontend/src/ErrorBoundary.test.tsxfrontend/src/ErrorBoundary.tsxfrontend/src/MaintainerAnalyticsPage.test.tsxfrontend/src/MaintainerAnalyticsPage.tsxfrontend/src/StatusFilterTabs.test.tsxfrontend/src/StatusFilterTabs.tsxfrontend/src/api.tsfrontend/src/index.cssfrontend/src/logger.tsfrontend/src/recommendations.tsfrontend/src/toast.test.tsxfrontend/src/utils.tsfrontend/tsconfig.jsonfrontend/tsconfig.node.tsbuildinfofrontend/tsconfig.tsbuildinfofrontend/vite.config.d.tsfrontend/vite.config.jsfrontend/vite.config.tsscripts/seed-bounties.js
💤 Files with no reviewable changes (1)
- frontend/src/utils.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (16)
.github/workflows/soroban-contract-ci.yml (2)
42-48:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick win🧩 Analysis chain
🌐 Web query:
Is actions-rs/toolchain archived? What is the recommended replacement for GitHub Actions Rust workflows?💡 Result:
Yes, the actions-rs/toolchain GitHub Action is officially archived and considered unmaintained [1][2][3][4]. It has not received active updates for several years, leading to compatibility issues such as the use of deprecated Node.js versions and legacy GitHub Actions commands [5][2][6]. The industry-standard recommendation for replacing actions-rs/toolchain in GitHub Actions workflows is dtolnay/rust-toolchain [5][6][7][3]. This action is widely used, actively maintained, and provides a concise way to install and configure Rust toolchains [7][3]. For most Rust projects, you can replace your existing actions-rs/toolchain setup with dtolnay/rust-toolchain as follows: Example replacement: - uses: dtolnay/rust-toolchain@stable with: components: clippy, rustfmt This provides equivalent functionality for installing the toolchain, setting overrides, and adding components like clippy and rustfmt [6][7][3]. Other community-maintained alternatives also exist, such as step-security/actions-rs-toolchain, which markets itself as a secure drop-in replacement, but dtolnay/rust-toolchain remains the most common and recommended choice [8][3].
Citations:
- 1: https://github.com/actions-rs/toolchain
- 2: actions-rs/toolchain#219
- 3: https://rustprojectprimer.com/ci/github.html
- 4: DioxusLabs/dioxus#1360
- 5: actions-rs/toolchain#221
- 6: actions-rs/toolchain#224
- 7: https://github.com/dtolnay/rust-toolchain
- 8: https://github.com/step-security/actions-rs-toolchain
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE=".github/workflows/soroban-contract-ci.yml" if [ ! -f "$FILE" ]; then echo "Missing file: $FILE" >&2 exit 1 fi nl -ba "$FILE" | sed -n '30,70p'Repository: ritik4ever/stellar-bounty-board
Length of output: 119
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE=".github/workflows/soroban-contract-ci.yml" awk 'NR>=30 && NR<=70 {printf "%5d %s\n", NR, $0}' "$FILE"Repository: ritik4ever/stellar-bounty-board
Length of output: 1656
🌐 Web query:
dtolnay/rust-toolchain action inputs override targets components profile💡 Result:
The dtolnay/rust-toolchain GitHub Action is designed for concise, automated installation of Rust toolchains using rustup [1][2]. Based on its current documentation and implementation, here is how it handles the requested parameters: Inputs: - toolchain: Optional. Defines the Rust toolchain (e.g., stable, nightly, or a specific version) [1][2]. If omitted, the action defaults to the toolchain associated with the action's
@rev(the git tag or branch used) [1]. - targets: Optional. A comma-separated list of additional targets (e.g., wasm32-unknown-unknown) to install [1][2]. - components: Optional. A comma-separated list of additional components (e.g., clippy, rustfmt) to install [1][2]. Regarding "override" and "profile": - profile: The dtolnay/rust-toolchain action does not provide an explicit input for setting the rustup profile (e.g., minimal, default, complete). It internally uses the --profile minimal flag during the rustup toolchain install command [3]. - override: The action does not provide an override input. It is designed to install the toolchain and make it available in the environment [1]. If you need to force a specific toolchain to be the active one for a directory (matching the behavior of rustup override), you can use the action's outputs to do so manually in subsequent steps: - uses: dtolnay/rust-toolchain@master id: toolchain with: toolchain: nightly - run: rustup override set ${{ steps.toolchain.outputs.name }} As of the current implementation, this action does not automatically parse or respect project-level rust-toolchain.toml files for its own configuration [4]. The workflow YML file remains the primary source of truth for the toolchain version and components installed by the action [4].Citations:
- 1: https://github.com/dtolnay/rust-toolchain/blob/master/README.md
- 2: https://github.com/dtolnay/rust-toolchain
- 3: https://github.com/dtolnay/rust-toolchain/blob/master/action.yml
- 4: dtolnay/rust-toolchain#133
Replace archived
actions-rs/toolchain@v1withdtolnay/rust-toolchain.
actions-rs/toolchainis archived/unmaintained and can break on current GitHub Actions due to outdated runtime behavior. Switch this workflow step todtolnay/rust-toolchain(it doesn’t exposeoverride, and uses--profile minimalinternally):🔄 Recommended replacement
- - name: Install Rust toolchain & wasm target - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - target: wasm32-unknown-unknown - components: clippy - profile: minimal - override: true + - name: Install Rust toolchain & wasm target + uses: dtolnay/rust-toolchain@stable + with: + targets: wasm32-unknown-unknown + components: clippy📝 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.uses: dtolnay/rust-toolchain@stable with: targets: wasm32-unknown-unknown components: clippy🧰 Tools
🪛 actionlint (1.7.12)
[error] 42-42: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.25.2)
[error] 42-42: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 42-42: action or reusable workflow from archived repository (archived-uses): repository is archived
(archived-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/soroban-contract-ci.yml around lines 42 - 48, The workflow step currently uses the archived action identifier "uses: actions-rs/toolchain@v1" and passes fields like "override: true" and "profile: minimal"; replace that step to use "dtolnay/rust-toolchain" instead, remove the unsupported "override" input and avoid duplicating "--profile minimal" since dtolnay/rust-toolchain applies minimal profile internally, and ensure the "toolchain: stable" and "target: wasm32-unknown-unknown" (and "components: clippy" if still needed) are passed according to dtolnay/rust-toolchain's inputs.
66-80:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix non-portable
statcommand for cross-platform compatibility.Line 70 uses
stat -c%s, which is Linux-specific and will fail on macOS runners. While this workflow runs onubuntu-latest, fixing portability improves maintainability and allows future runner changes.🔧 Portable alternative
WASM_FILE=$(ls contracts/target/wasm32-unknown-unknown/release/*.wasm 2>/dev/null || true) if [ -n "$WASM_FILE" ]; then echo "### WASM binary size" >> $GITHUB_STEP_SUMMARY - SIZE=$(stat -c%s "$WASM_FILE") + SIZE=$(wc -c < "$WASM_FILE" | tr -d ' ') echo "- File: $WASM_FILE" >> $GITHUB_STEP_SUMMARY echo "- Size (bytes): $SIZE" >> $GITHUB_STEP_SUMMARY GZ_SIZE=$(gzip -c "$WASM_FILE" | wc -c)The
wc -c < filepattern is POSIX-compliant and works on both Linux and macOS.🤖 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/soroban-contract-ci.yml around lines 66 - 80, Replace the Linux-specific stat usage that computes SIZE (the line using stat -c%s "$WASM_FILE") with a POSIX-portable approach; read the WASM_FILE variable and compute size with a portable wc invocation (e.g., use wc -c < "$WASM_FILE" or similar) so the SIZE calculation in the section that prints WASM binary size works on macOS and Linux runners; update the SIZE assignment and any references that echo "$SIZE" accordingly while keeping the existing WASM_FILE, GZ_SIZE, and GITHUB_STEP_SUMMARY logic intact.backend/src/app.ts (1)
502-504:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
listOpenIssues()rejections in/api/open-issues.If
listOpenIssues()throws (e.g., GitHub fetch fails and cache is empty), the async Express 4 handler won’t reliably route it throughsendError, so the endpoint can fail as an unhandled rejection/non-JSON response.Suggested fix
-app.get('/api/open-issues', async (_req: Request, res: Response) => { - res.setHeader('Cache-Control', 'max-age=600'); - res.json({ data: await listOpenIssues() }); +app.get('/api/open-issues', async (req: Request, res: Response) => { + try { + const issues = await listOpenIssues(); + res.setHeader('Cache-Control', 'max-age=600'); + res.json({ data: issues }); + } catch (error) { + sendError(res, req, error, 502); + } });📝 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.app.get('/api/open-issues', async (req: Request, res: Response) => { try { const issues = await listOpenIssues(); res.setHeader('Cache-Control', 'max-age=600'); res.json({ data: issues }); } catch (error) { sendError(res, req, error, 502); } });🤖 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 `@backend/src/app.ts` around lines 502 - 504, The /api/open-issues route calls listOpenIssues() without handling rejections; wrap the await listOpenIssues() call in a try/catch inside the async handler (app.get('/api/open-issues', ...)) and on error call the project's error responder (e.g., sendError(res, err) or next(err) depending on existing convention) so errors are returned as JSON instead of causing an unhandled rejection; on success keep setting Cache-Control and res.json({ data }) as before.backend/src/metrics.ts (1)
1-17:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== File: backend/src/metrics.ts ==" if [ -f backend/src/metrics.ts ]; then nl -ba backend/src/metrics.ts | sed -n '1,220p' else echo "MISSING: backend/src/metrics.ts" fi echo echo "== Find references to metrics.ts and /api/metrics ==" rg -n "from ['\"]\.?/metrics['\"]|require\(['\"]\.?/metrics['\"]\)|/api/metrics|metrics\(\)" backend/src || true echo echo "== Show backend/src/app.ts (or likely server entry) ==" for f in backend/src/app.ts backend/src/server.ts backend/src/index.ts backend/src/main.ts; do if [ -f "$f" ]; then echo "--- $f ---" nl -ba "$f" | sed -n '1,260p' fi done echo echo "== Show any env/test conditionals around metrics (search) ==" rg -n "NODE_ENV|test|__tests__|process\.env|metrics" backend/src || true echo echo "== Prom-client usage check ==" rg -n "prom-client" backend/src || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 171
🏁 Script executed:
#!/bin/bash set -euo pipefail print_file() { local f="$1" echo "== $f ==" if [ -f "$f" ]; then awk '{printf "%d:%s\n", NR, $0}' "$f" | sed -n '1,220p' else echo "MISSING: $f" fi echo } print_file backend/src/metrics.ts echo "== Find references to /api/metrics and metrics.ts usage ==" rg -n "/api/metrics|backend/src/metrics|metrics\\.ts|collectDefaultMetrics\\(|new Registry\\(|prom-client|getSingleMetric\\(" backend/src || true echo echo "== Show likely server entry files that register routes ==" for f in backend/src/app.ts backend/src/server.ts backend/src/index.ts backend/src/main.ts backend/src/routes.ts backend/src/router.ts; do [ -f "$f" ] && print_file "$f" done echo "== Look for env/test conditionals around metrics ==" rg -n "NODE_ENV|process\\.env|test\\b|__tests__|metrics" backend/src || true echo echo "== Check if prom-client is present anywhere ==" rg -n "prom-client" backend/src || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 16928
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Usage of exported metrics in backend/src ==" rg -n "bountiesCreatedTotal\\.|bountiesReleasedTotal\\.|bountiesDisputedTotal\\.|httpRequestDuration\\." backend/src || true echo echo "== Calls to .inc( or .observe( on metrics instances ==" rg -n "\\.inc\\(|\\.observe\\(" backend/src/services backend/src | head -n 200 || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 723
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Inspect bountiesCreatedTotal.inc call context ==" awk 'NR>=520 && NR<=620 {printf "%5d:%s\n", NR, $0}' backend/src/services/bountyStore.ts echo echo "== Inspect bountiesReleasedTotal.inc call context ==" awk 'NR>=710 && NR<=820 {printf "%5d:%s\n", NR, $0}' backend/src/services/bountyStore.ts echo echo "== Inspect httpRequestDuration.observe call context (app.ts) ==" awk 'NR>=40 && NR<=120 {printf "%5d:%s\n", NR, $0}' backend/src/app.tsRepository: ritik4ever/stellar-bounty-board
Length of output: 11372
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "prom-client" . || true ls -la for f in package.json backend/package.json; do if [ -f "$f" ]; then echo "== $f =="; sed -n '1,200p' "$f" fi doneRepository: ritik4ever/stellar-bounty-board
Length of output: 6907
🏁 Script executed:
#!/bin/bash set -euo pipefail awk 'NR>=500 && NR<=590 {printf "%5d:%s\n", NR, $0}' backend/src/app.tsRepository: ritik4ever/stellar-bounty-board
Length of output: 2201
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "prom-client" package-lock.json backend/package-lock.json package.json backend/package.json 2>/dev/null || true fd -a "metrics.ts" backend -t f 2>/dev/null || true rg -n "from ['\"]\\.\\/metrics['\"]|from ['\"]\\.\\.\\/metrics['\"]|getMetrics\\(" backend/src || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 277
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Search for prom-client in repo ==" rg -n --fixed-string "prom-client" . || true echo echo "== Search for getMetrics() usage ==" rg -n --fixed-string "getMetrics(" backend/src || true echo echo "== Search for imports of ./metrics or ../metrics ==" rg -n --glob 'backend/src/**/*.ts' "from ['\"][.]{0,2}/metrics['\"]" backend/src || true echo echo "== List any *metrics* files ==" fd -t f -a "*metrics*" . | head -n 200Repository: ritik4ever/stellar-bounty-board
Length of output: 1631
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Search for prom-client in repo ==" rg -n --fixed-strings "prom-client" . || true echo echo "== Search for getMetrics() usage in backend ==" rg -n --fixed-strings "getMetrics(" backend/src || true echo echo "== List any metrics-related files ==" fd -t f --glob '*metrics*' backend src . || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 457
Production metrics are fully disabled:
/api/metricsalways returns an empty string.
backend/src/metrics.tsreplacesprom-clientwith stub implementations whereRegistry.metrics()returns'',getSingleMetric()always returnsnull, andCounter.inc()/Histogram.observe()are no-ops. The/api/metricsroute inbackend/src/app.tsunconditionally returnsgetMetrics()(no env gating), while the app still callshttpRequestDuration.observe(...)on every request and increments bounty counters—so Prometheus scraping/alerts silently stop working in all environments.Suggested fix
-class Registry { - metrics(): string { return ''; } - getSingleMetric(name: string): any { return null; } -} - -class Counter<T extends string = string> { - constructor(options: any) {} - inc(labels?: any, value?: number): void {} - observe(labels: any, value: number): void {} -} - -class Histogram<T extends string = string> { - constructor(options: any) {} - observe(labels: any, value: number): void {} -} - -function collectDefaultMetrics(options: any): void {} +import { collectDefaultMetrics, Counter, Histogram, Registry } from 'prom-client';Also add
prom-clienttobackend/package.json(it’s currently not in dependencies).🧰 Tools
🪛 ESLint
[error] 3-3: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)
[error] 7-7: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)
[error] 8-8: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)
[error] 9-9: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)
[error] 13-13: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)
[error] 14-14: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)
[error] 17-17: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)🤖 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 `@backend/src/metrics.ts` around lines 1 - 17, Replace the stubs in metrics.ts with the real prom-client implementations: import Registry, Counter, Histogram and collectDefaultMetrics from 'prom-client' and re-export them (so Registry.metrics() and getSingleMetric() behave correctly and Counter.inc/Histogram.observe actually record values); update the /api/metrics usage in app (the getMetrics() call) to return real registry.metrics() output and add an environment flag (e.g., METRICS_ENABLED) so metrics are only exposed when enabled; finally add 'prom-client' to backend/package.json dependencies.backend/src/services/cache.ts (1)
1-7:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Redis cache backend: remove the inline
Redisstub.In
backend/src/services/cache.ts(lines 1-7), the localclass Redisstub makesget()always returnnulland makesset()/del()no-ops. WhenREDIS_URLis set,getCache()still selectsRedisCache, so production effectively runs the “redis” path while never persisting or invalidating—despite loggingbackend: "redis". Replace the stub with the realioredisclient (matching the project’s existing import/style) soRedisCachecan perform actualget/set/del.🧰 Tools
🪛 ESLint
[error] 2-2: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)
[error] 3-3: Don't use
Functionas a type. TheFunctiontype accepts any function-like value.
It provides no type safety when calling the function, which can be a common source of bugs.
It also accepts things like class declarations, which will throw at runtime as they will not be called withnew.
If you are expecting the function to accept certain arguments, you should explicitly define the function shape.(
@typescript-eslint/ban-types)🤖 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 `@backend/src/services/cache.ts` around lines 1 - 7, The local stub class Redis in backend/src/services/cache.ts must be removed and replaced with the real ioredis client import so RedisCache actually performs get/set/del; locate the stubbed "class Redis" definition and delete it, add an import of the project's ioredis client (matching existing project style) and ensure RedisCache and getCache() instantiate and use that ioredis client so methods like RedisCache.get, RedisCache.set and RedisCache.del call the real client's get/set/del instead of the no-op stub.backend/test/webhookSecretValidation.test.ts (1)
11-13:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMove
vi.mock()to module scope for proper hoisting.Vitest requires
vi.mock()calls to be at the top level of the module (outside anydescribe,beforeEach, or other blocks) for proper hoisting. The current placement insidebeforeEachwill not correctly mock the logger module.🔧 Proposed fix
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { validateGitHubWebhookSecret } from "../src/validation/webhookSecretValidation"; +// Mock the logger to avoid console output during tests +vi.mock("../src/logger", () => ({ + logStructured: vi.fn(), +})); + describe("validateGitHubWebhookSecret", () => { const originalEnv = process.env; beforeEach(() => { // Create a fresh copy of environment variables for each test process.env = { ...originalEnv }; - // Mock the logger to avoid console output during tests - vi.mock("../src/logger", () => ({ - logStructured: vi.fn(), - })); }); afterEach(() => {📝 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.import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { validateGitHubWebhookSecret } from "../src/validation/webhookSecretValidation"; // Mock the logger to avoid console output during tests vi.mock("../src/logger", () => ({ logStructured: vi.fn(), })); describe("validateGitHubWebhookSecret", () => { const originalEnv = process.env; beforeEach(() => { // Create a fresh copy of environment variables for each test process.env = { ...originalEnv }; }); afterEach(() => { process.env = originalEnv; vi.clearAllMocks(); }); });🤖 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 `@backend/test/webhookSecretValidation.test.ts` around lines 11 - 13, The vi.mock("../src/logger", () => ({ logStructured: vi.fn() })) call is currently inside beforeEach and must be moved to module scope so Vitest can hoist it; relocate that vi.mock invocation to the top of the test file (above any imports or describe blocks) to ensure the logger module is mocked before it's imported/used, keeping the mocked export name logStructured unchanged.docs/ARCHITECTURE.md (1)
232-263:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winReconcile state machine inconsistencies across diagrams.
The new Mermaid diagram conflicts with the ASCII diagram and transition rules table:
Disputed state missing from ASCII: The Mermaid diagram introduces a
Disputedstate (lines 247-252) that is documented in the sequence diagram (lines 156-173) but completely absent from the ASCII state diagram (lines 176-230).Submitted → Refunded conflict: Line 248 shows
Submitted --> Refunded : refund_bounty, but the ASCII transition rules explicitly stateSUBMITTED → (no refund): Submitted bounties must be reviewed(line 228).Please update all three representations (ASCII diagram, transition table, and Mermaid diagram) to present a consistent state machine. If the Disputed state and Submitted→Refunded transition are valid, add them to the ASCII diagram and update line 228. If they are not valid, remove them from the Mermaid diagram.
🤖 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 `@docs/ARCHITECTURE.md` around lines 232 - 263, The state machine docs are inconsistent: reconcile the Mermaid diagram, ASCII diagram, and transition table so they all match; decide whether the Disputed state and the transition Submitted --> Refunded (refund_bounty) are valid, then apply the chosen design everywhere—if Disputed and Submitted→Refunded are valid, add "Disputed" and the Submitted -> Refunded (refund_bounty) transition to the ASCII state diagram and update the transition rules table entry that currently forbids refunds for SUBMITTED; otherwise remove the Disputed state and the Submitted -> Refunded line (refund_bounty) from the Mermaid diagram and any sequence diagrams so all three representations are identical.frontend/src/App.tsx (2)
351-370:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop treating a failed metrics fetch as an endless loading state.
Lines 351-370 clear
maintainerMetricson rejection, and Lines 864-865 render"Loading metrics..."whenever that value isnull. A 404 or network failure therefore leaves this route stuck forever with no error or retry path.Proposed fix
+ const [maintainerMetricsError, setMaintainerMetricsError] = useState<string | null>(null); + useEffect(() => { if (!maintainerAddress) { setMaintainerMetrics(null); + setMaintainerMetricsError(null); return; } let active = true; setMaintainerMetricsLoading(true); + setMaintainerMetricsError(null); getMaintainerMetrics(maintainerAddress) .then((data) => { if (active) { setMaintainerMetrics(data); + setMaintainerMetricsError(null); setMaintainerMetricsLoading(false); } }) - .catch(() => { + .catch((err) => { if (active) { setMaintainerMetrics(null); + setMaintainerMetricsError(err instanceof Error ? err.message : "Failed to load maintainer metrics."); setMaintainerMetricsLoading(false); } }); @@ - {maintainerMetricsLoading || !maintainerMetrics ? ( + {maintainerMetricsLoading ? ( <div className="empty-state">Loading metrics...</div> + ) : maintainerMetricsError ? ( + <div className="error-banner">{maintainerMetricsError}</div> + ) : !maintainerMetrics ? ( + <div className="empty-state">No metrics available for this maintainer.</div> ) : (Also applies to: 864-865
🤖 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 `@frontend/src/App.tsx` around lines 351 - 370, The effect treating a failed getMaintainerMetrics call as a permanent "null = loading" is causing endless "Loading metrics..." UI; add a separate error state (e.g., maintainerMetricsError via useState) and update the effect using getMaintainerMetrics so that on success you setMaintainerMetrics(data), setMaintainerMetricsError(null) and setMaintainerMetricsLoading(false), and on catch you setMaintainerMetricsError(error or true) and setMaintainerMetricsLoading(false) (do NOT leave maintainerMetrics null to signal loading). Then update the render logic that currently shows "Loading metrics..." when maintainerMetrics is null to instead show loading only when maintainerMetricsLoading is true and show an error/ retry UI when maintainerMetricsError is set; reference getMaintainerMetrics, setMaintainerMetrics, setMaintainerMetricsLoading, and maintainerMetrics state when making these changes.
835-876:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
/maintainer/:addresswith the connected wallet state.Lines 835-876 render the analytics page for any
/maintainer/:addressURL, and Lines 983-993 only clear local storage on disconnect. After disconnecting, the current analytics page remains fully accessible, which misses the connect-wallet gating described in the PR objectives.Proposed fix
if (maintainerAddress) { + if (!connectedWallet || connectedWallet !== maintainerAddress) { + return ( + <div className="page-shell"> + <div className="glow glow-left" /> + <div className="glow glow-right" /> + <div className="empty-state"> + Connect the matching wallet to view maintainer analytics. + </div> + </div> + ); + } + return ( <div className="page-shell">Also applies to: 983-993
🤖 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 `@frontend/src/App.tsx` around lines 835 - 876, The maintainer analytics view is currently shown for any /maintainer/:address even when the user is disconnected; update the render guard so that MaintainerAnalyticsPage (and the nav "Analytics Dashboard" button) only render when connectedWallet exists and equals maintainerAddress (use connectedWallet === maintainerAddress), otherwise show a gated state (e.g., prompt to connect or navigate("/") ) so unauthorized viewers cannot access the page; also update the disconnect logic that clears local storage (the logic around the disconnect handler) to additionally navigate away or clear maintainerAddress state when the wallet is disconnected so the gated view is enforced after disconnect.frontend/src/ContributorProfilePage.tsx (2)
55-71:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the metadata writer before this page ships.
Lines 58-70 always create
meta[name="..."], but Open Graph tags likeog:titlemust useproperty="og:title". The effect also never restores prior values on unmount, so contributor-specific title/meta can leak into later SPA routes.Proposed fix
useEffect(() => { const title = `Contributor ${shortAddress(address)} — Stellar Bounty Board`; + const previousTitle = document.title; document.title = title; - const setMeta = (name: string, content: string) => { - let el = document.querySelector(`meta[name="${name}"]`) as HTMLMetaElement | null; + const setMeta = (key: string, content: string, attr: "name" | "property" = "name") => { + let el = document.querySelector(`meta[${attr}="${key}"]`) as HTMLMetaElement | null; + const created = !el; if (!el) { el = document.createElement("meta"); - el.setAttribute("name", name); + el.setAttribute(attr, key); document.head.appendChild(el); } el.content = content; + return () => { + if (created) { + el?.remove(); + } + }; }; - setMeta("description", `Profile for contributor ${shortAddress(address)} — earned reputation and completed work.`); - setMeta("twitter:card", "summary"); - setMeta("twitter:title", title); - setMeta("og:title", title); + const cleanups = [ + setMeta("description", `Profile for contributor ${shortAddress(address)} — earned reputation and completed work.`), + setMeta("twitter:card", "summary"), + setMeta("twitter:title", title), + setMeta("og:title", title, "property"), + ]; + + return () => { + document.title = previousTitle; + cleanups.forEach((cleanup) => cleanup()); + }; }, [address]);🤖 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 `@frontend/src/ContributorProfilePage.tsx` around lines 55 - 71, The metadata updater in the useEffect (ContributorProfilePage -> setMeta) incorrectly creates all tags as meta[name="..."] (breaking Open Graph which requires property="...") and never restores previous title/meta on unmount, causing leaks across SPA routes; change setMeta to accept whether the key is an OG/property tag (detect keys starting with "og:" or "twitter:" as needed) and query/create the element using property="..." for og: keys and name="..." for others, store the previous document.title and previous meta content (or whether the tag was newly created) when mounting, update document.title via document.title = title, and in the useEffect cleanup restore the saved title and either restore previous meta content or remove any meta elements you created (use the unique symbol names: useEffect, setMeta, shortAddress, document.title) so contributor-specific metadata doesn't leak after unmount.
73-81:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't collapse mixed-token earnings into a single
XLMtotal.Lines 73-81 sum every released bounty into one
totalEarned, and Lines 97-100 label the result asXLM. If a contributor has payouts in multiple assets, this reports financially incorrect data.Also applies to: 97-100
🤖 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 `@frontend/src/ContributorProfilePage.tsx` around lines 73 - 81, The stats calculation collapses all released bounties into a single numeric total (totalEarned) and the UI labels it as "XLM", which is wrong for multi-asset payouts; change the calc in the useMemo (stats) to aggregate released amounts by asset (e.g., produce totalEarned as a map/object like { [asset]: sum }) by summing b.amount grouped by b.asset (fallback to a default asset if missing), keep released as-is, and update any UI that reads totalEarned (and the component that currently renders the "XLM" label) to iterate over the asset map and render each asset-specific total instead of assuming XLM.frontend/src/logger.ts (1)
1-13:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Check if pino is declared in frontend package.json cat frontend/package.json | jq '.dependencies.pino, .devDependencies.pino'Repository: ritik4ever/stellar-bounty-board
Length of output: 85
Add/resolve missing
pinodependency forfrontend/src/logger.ts
frontend/package.jsondoes not declarepino(both.dependencies.pinoand.devDependencies.pinoarenull), whilefrontend/src/logger.tsimportspino; ensurepinois declared in the package.json that powers this frontend build. Also consider a browser-appropriate logging approach (console, orpino-browserif you need the pino API).🤖 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 `@frontend/src/logger.ts` around lines 1 - 13, The frontend imports pino in logger.ts (const logger = pino(...)) and exposes logError/logger, but the frontend package.json lacks pino; add pino to the frontend's dependencies (or replace the import with a browser-friendly logger) so builds don't break: either install "pino" into the frontend package.json dependencies and keep logger.ts as-is, or swap the implementation to use console (or import "pino-browser"/an equivalent) and update the import in logger.ts (logError and logger) so the bundler/runtime use a browser-compatible logger.frontend/src/recommendations.ts (2)
35-38:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash echo '--- Bounty interface ---' fd -a 'types.ts' frontend/src | while read -r file; do sed -n '/export interface Bounty/,/^[}]/p' "$file" done echo '--- tag references in frontend/src ---' rg -n -C2 '\btags\b' frontend/src --glob '*.{ts,tsx}'Repository: ritik4ever/stellar-bounty-board
Length of output: 4677
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the scoreMatch implementation and the referenced logic area echo "=== frontend/src/recommendations.ts (lines 1-140) ===" nl -ba frontend/src/recommendations.ts | sed -n '1,140p' echo "=== substring-ish comparisons inside recommendations.ts ===" rg -n "\.includes\(|indexOf\(|toLowerCase\(\).*includes|includes.*toLowerCase" frontend/src/recommendations.ts || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 190
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== frontend/src/recommendations.ts (lines 1-140) ===" if command -v cat >/dev/null 2>&1; then # cat -n should exist cat -n frontend/src/recommendations.ts | sed -n '1,140p' else echo "cat not found" >&2 exit 1 fi echo "=== likely substring matching patterns in recommendations.ts ===" rg -n "\.includes\(" frontend/src/recommendations.ts || true rg -n "indexOf\(" frontend/src/recommendations.ts || true rg -n "toLowerCase\(\)" frontend/src/recommendations.ts || true rg -n "includes.*toLowerCase|toLowerCase.*includes" frontend/src/recommendations.ts || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 5997
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== scoreMatch usages ===" rg -n "scoreMatch\(" frontend/src --glob '*.{ts,tsx}' echo "=== inspect around scoreMatch caller(s) (first few hits) ===" # Print nearby sections for the first couple of matches rg -n "scoreMatch\(" frontend/src --glob '*.{ts,tsx}' -m 5 | cut -d: -f1-2 | while read -r file line; do echo "--- $file:$line ---" start=$((line-25)) end=$((line+60)) sed -n "${start},${end}p" "$file" | nl -ba -w2 -s': ' 2>/dev/null || true done echo "=== other substring-like matching in recommendations.ts beyond scoreMatch ===" rg -n "\.includes\(|indexOf\(|skillKeywords\.includes|token\.includes" frontend/src/recommendations.tsRepository: ritik4ever/stellar-bounty-board
Length of output: 10342
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== bounty.tags references ===" rg -n "bounty\.tags" frontend/src --glob '*.{ts,tsx}' || true echo "=== RecommendedBounties references (file existence / related usage) ===" fd -a '*RecommendedBounties*' frontend/src || true rg -n "RecommendedBounties" frontend/src --glob '*.{ts,tsx}' || trueRepository: ritik4ever/stellar-bounty-board
Length of output: 1092
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== RecommendedBounties.tsx: mentions of tags ===" rg -n "\btags\b" frontend/src/RecommendedBounties.tsx || true echo "=== recommendations.ts: calculateRecommendationScore excerpt (skillScore + reasons) ===" cat -n frontend/src/recommendations.ts | sed -n '130,240p' echo "=== recommendations.ts: any further matching affecting reasons ===" cat -n frontend/src/recommendations.ts | sed -n '240,360p'Repository: ritik4ever/stellar-bounty-board
Length of output: 5350
Remove the unsafe
bounty.tagscasting and tighten the matching logic inscoreMatch.
frontend/src/recommendations.tsalready hasBounty.tags?: string[]intypes.ts, so lines 36-38 don’t need(bounty as unknown as Record<string, unknown>).tags—usebounty.tagsdirectly to keep type safety (and avoid silently accepting non-string payloads).scoreMatchlines 69-72 uses broad substring matching (token.includes(skill) || skill.includes(token)), which can easily over-match short/common skills and inflate the recommendation score (and the “Matches X% of your skills” reason) based on incidental text overlap.🤖 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 `@frontend/src/recommendations.ts` around lines 35 - 38, Replace the unsafe cast and tighten matching: stop using (bounty as unknown as Record<string, unknown>).tags and reference the typed property bounty.tags directly (Bounty.tags?: string[]) in frontend/src/recommendations.ts; then revise the scoreMatch function so it does exact token equality or normalized word-boundary matching instead of the broad substring checks (replace token.includes(skill) || skill.includes(token) with an exact match or regex/word-split comparison after lowercasing and trimming) to avoid over-matching and inflate scores.
56-72:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove substring matching for free-text tokens.
Line 71 lets short skills match unrelated words from titles and summaries, so skills like
go,js, oraiwill score against tokens such asgood,adjust, orpaid. That inflates both ranking and theMatches X% of your skillsreason for unrelated bounties. Normalize both sides to boundary-separated tokens and only compare exact terms, or at least gate the fallback to longer terms.🤖 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 `@frontend/src/recommendations.ts` around lines 56 - 72, The current matching loop in recommendations.ts (using normalizedSkills, expandedTokens, and matchCount) must stop using substring checks (token.includes(skill) || skill.includes(token)) because short skills like "go" or "ai" match unrelated words; replace that logic so you only count matches when a normalized skill exactly equals a normalized token (compare exact terms), or if you need a fallback keep it guarded: only allow substring fallback when both the skill and token lengths are >= 3 (or another chosen threshold) and both sides are normalized/boundary-tokenized; update the loop that increments matchCount to use exact equality (and the optional guarded fallback) instead of unguarded includes.frontend/vite.config.js (1)
19-37:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win🧩 Analysis chain
🌐 Web query:
DoesContent-Security-Policy-Report-Onlywork when delivered via atag, or must it be sent as an HTTP response header?💡 Result:
The Content-Security-Policy-Report-Only policy cannot be delivered via a meta http-equiv tag; it must be sent as an HTTP response header [1][2][3]. According to the Content Security Policy (CSP) specification, the meta element only supports the standard Content-Security-Policy header [1][4]. Attempts to use Content-Security-Policy-Report-Only within a meta tag are not supported by browsers and will be ignored [2][5][3]. Furthermore, certain directives—such as frame-ancestors, report-uri, report-to, and sandbox—are also unsupported when using the meta tag delivery method for any CSP policy [1][4][5]. If you need to use report-only mode, you must configure your web server or application to send the Content-Security-Policy-Report-Only HTTP response header [6][3].
Citations:
- 1: https://w3c.github.io/webappsec-csp/
- 2: https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/CSP
- 3: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Security-Policy-Report-Only
- 4: https://www.w3.org/TR/CSP3/
- 5: https://csplite.com/csp67/
- 6: https://www.invicti.com/web-application-vulnerabilities/content-security-policy-report-only-cannot-be-declared-between-meta-tags
Move CSP
Report-Onlyout oftransformIndexHtml(send as an HTTP response header).
Content-Security-Policy-Report-Onlyis not supported via<meta http-equiv="...">; browsers ignore it. If you need report-only rollout, configure your hosting/app layer to send theContent-Security-Policy-Report-OnlyHTTP header, and limit any HTML meta injection to an enforcingContent-Security-Policyonly. https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Security-Policy-Report-Only🤖 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 `@frontend/vite.config.js` around lines 19 - 37, The plugin currently injects a Content-Security-Policy-Report-Only meta tag from cspPlugin.transformIndexHtml (variable reportOnlyMeta) which browsers ignore; remove the Report-Only meta injection there and either inject an enforcing Content-Security-Policy meta (use "Content-Security-Policy" instead of "Content-Security-Policy-Report-Only") or better yet stop meta injection altogether and configure your hosting/app layer or Vite dev server to send the "Content-Security-Policy-Report-Only" HTTP response header (e.g., add a middleware or server.headers config) so report-only rollout is done via the HTTP header while any in-HTML meta remains enforcing if needed.frontend/vite.config.ts (1)
4-10:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -e python - <<'PY' from pathlib import Path text = Path("frontend/vite.config.ts").read_text().splitlines() print("Top of frontend/vite.config.ts:") for i, line in enumerate(text[:15], 1): print(f"{i}: {line}") print({ "has_defineConfig_import": any('defineConfig' in line and 'from "vite"' in line for line in text), "has_VitePWA_import": any('VitePWA' in line and 'from "vite-plugin-pwa"' in line for line in text), "has_stray_ain_token": any(line.strip() == "ain" for line in text), }) PYRepository: ritik4ever/stellar-bounty-board
Length of output: 598
Fix
frontend/vite.config.tsso Vite config parses and starts (remove strayain, add missing imports).
- Line 5 contains a stray
aintoken, breaking TS parsing.defineConfigandVitePWAare referenced but not imported, causing config startup failure.Suggested fix
+import { defineConfig } from "vite"; import react from "`@vitejs/plugin-react`"; -ain +import { VitePWA } from "vite-plugin-pwa";📝 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.import { defineConfig } from "vite"; import react from "`@vitejs/plugin-react`"; import { VitePWA } from "vite-plugin-pwa"; export default defineConfig({ plugins: [ react(), VitePWA({🤖 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 `@frontend/vite.config.ts` around lines 4 - 10, Remove the stray token "ain" (it's breaking TS parsing) and add the missing imports: import { defineConfig } from "vite" and import { VitePWA } from "vite-plugin-pwa"; keep the existing react import (import react from "`@vitejs/plugin-react`") and ensure defineConfig(...) and VitePWA(...) are referenced correctly inside the default export.
- vite.config.ts: remove stray 'ain' token, add missing defineConfig and VitePWA imports - logger.ts: replace pino (not browser-compatible) with console-based logger - metrics.ts: replace stubs with real prom-client imports; add prom-client to backend deps - cache.ts: replace inline Redis stub with real ioredis import; add ioredis to backend deps - webhookSecretValidation.test.ts: move vi.mock() to module scope for proper hoisting - gitleaks.yml: fix action version v8 -> v2 (v8 does not exist) - vite-plugin-pwa: installed in frontend deps
- .npmrc: add legacy-peer-deps=true to resolve eslint-import-resolver-typescript peer conflict
- ci.yml: pin cargo-audit to 0.21.2 to avoid panic on indexmap 2.13.0 (bug in 0.22.1)
- gitleaks.yml: add required GITHUB_TOKEN env, remove unsupported args input
- docker-publish.yml: fix invalid tag by replacing {{branch}}- prefix with static sha-
- contracts/Cargo.lock: remove duplicate heapless and heck entries causing cargo clippy parse error
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/logger.ts (1)
2-3: ⚡ Quick winPreserve object details for non-
Errorfailures.
String(error)turns plain objects into[object Object], which drops the payload you need to debug API and promise failures. Serialize object values explicitly before logging them.♻️ Proposed fix
export function logError(component: string, error: unknown) { - const message = error instanceof Error ? error.message : String(error); + const message = + error instanceof Error + ? error.message + : typeof error === "object" && error !== null + ? safeStringify(error) + : String(error); console.error(`[${component}]`, message, error instanceof Error ? error.stack : ""); } + +function safeStringify(value: unknown) { + try { + return JSON.stringify(value); + } catch { + return Object.prototype.toString.call(value); + } +}🤖 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 `@frontend/src/logger.ts` around lines 2 - 3, The current logger converts non-Error values with String(error) which collapses objects to "[object Object]"; change the logic that assigns message (and/or what is passed to console.error) to serialize non-Error objects (e.g., use JSON.stringify with a safe/circular-aware fallback inside a try/catch) so object payloads are preserved when logging; update the lines that set the message variable and the console.error call to use the serialized representation for non-Error values while keeping Error.stack for Error instances.
🤖 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.
Nitpick comments:
In `@frontend/src/logger.ts`:
- Around line 2-3: The current logger converts non-Error values with
String(error) which collapses objects to "[object Object]"; change the logic
that assigns message (and/or what is passed to console.error) to serialize
non-Error objects (e.g., use JSON.stringify with a safe/circular-aware fallback
inside a try/catch) so object payloads are preserved when logging; update the
lines that set the message variable and the console.error call to use the
serialized representation for non-Error values while keeping Error.stack for
Error instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 784945ca-0558-4ab5-bee6-2f77b57b5c38
⛔ Files ignored due to path filters (3)
backend/package-lock.jsonis excluded by!**/package-lock.jsoncontracts/Cargo.lockis excluded by!**/*.lockfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/ci.yml.github/workflows/docker-publish.yml.github/workflows/gitleaks.yml.npmrcbackend/package.jsonbackend/src/index.tsbackend/src/metrics.tsbackend/test/webhookSecretValidation.test.tsfrontend/package.jsonfrontend/src/logger.tsfrontend/vite.config.ts
💤 Files with no reviewable changes (1)
- backend/src/index.ts
✅ Files skipped from review due to trivial changes (4)
- .npmrc
- .github/PULL_REQUEST_TEMPLATE.md
- backend/src/metrics.ts
- backend/test/webhookSecretValidation.test.ts
Closes #301
This PR builds the maintainer analytics dashboard page at
/maintainer/:addresswith charts, summary cards, and full test coverage.What was done:
/maintainer/:addresspageMaintainerMetricsfrom/api/maintainer/:address/metricson loadSummary cards
Bar chart — bounties by status
open,reserved,submitted,released,refunded,expiredBarChart; each status bar distinctly colouredLine chart — funded vs. released over time
LineChartwith time-axis labelsHeader nav
/maintainer/:addressusing the connected wallet addressVitest test
maintainerDashboard.test.tsxrenders the page with mockedMaintainerMetricsdataAcceptance criteria met:
Summary by CodeRabbit
New Features
/api/health/deependpoint for extended health checks.Improvements
UI/UX