Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for storing and updating Terms & Conditions dynamically (via admin UI + API + R2), and replaces the prior regex-based formatting with a shared Markdown renderer.
Changes:
- Introduce
/api/termsGET/PUT backed by Cloudflare R2, with ARC-60 (SIWA) signature verification for updates. - Add an Admin “Terms & Conditions” editor with preview + wallet-signing flow.
- Centralize markdown rendering via
markedand switch existing T&C views/modals to use the new source.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
wrangler.jsonc |
Adds R2 bucket bindings needed for storing versioned T&C content. |
vitest.config.ts |
Adds Vitest config (path alias) for new unit tests. |
src/recipes/admin/index.ts |
Exposes the new TermsEditor recipe export. |
src/recipes/admin/TermsEditor.tsx |
Admin UI for editing/previewing and saving T&C via ARC-60 signed request. |
src/recipes/admin/AdminPage.tsx |
Adds the Terms editor section to the admin panel. |
src/recipes/TermsAndConditions.tsx |
Switches modal rendering to shared markdown renderer. |
src/pages/api/terms.ts |
Implements T&C fetch + update endpoints, with R2 persistence and signature verification. |
src/lib/markdown.ts |
Adds shared markdown rendering helper used across app. |
src/lib/arc60.ts |
Adds ARC-60/SIWA challenge generation + verification utilities. |
src/lib/arc60.test.ts |
Adds unit tests for ARC-60 verification logic. |
src/hooks/useTerms.ts |
Adds React Query hook to fetch current terms. |
src/hooks/index.ts |
Re-exports the new useTerms hook. |
src/env.d.ts |
Adds Cloudflare R2 typing for locals.runtime.env. |
src/components/TutorialDialog/TutorialDialog.tsx |
Swaps tutorial modal terms source from bundled markdown to API-backed terms. |
src/components/TermsAndConditionsView/TermsAndConditionsView.tsx |
Switches full-page terms view to API-backed terms + markdown renderer. |
src/components/ProfileCard/ProfileCard.tsx |
Swaps proposer signup terms source from bundled markdown to API-backed terms. |
package.json |
Adds new deps (marked, tweetnacl, etc.) + unit test script. |
package-lock.json |
Locks dependency updates for the new packages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| { | ||
| "bucket_name": "xgov", | ||
| "binding": "xgov", | ||
| "remote": true | ||
| }, |
There was a problem hiding this comment.
This second R2 binding (binding: "xgov") doesn’t appear to be referenced by the worker (the code reads env.BUCKET). If it’s not needed, remove it to avoid configuration drift; if it is needed, align the binding name with what runtime code expects.
| }, | |
| { | |
| "bucket_name": "xgov", | |
| "binding": "xgov", | |
| "remote": true | |
| }, | |
| } |
| "tsx": "^4.19.3", | ||
| "vite-plugin-pwa": "^1.0.0", | ||
| "vite-tsconfig-paths": "^5.0.1", | ||
| "vitest": "^4.0.18", |
There was a problem hiding this comment.
vitest@^4 declares Node >= 20 in its engines, but this repo supports Node 18 (docs + CI matrix). That means npm run test:unit won’t work on Node 18 and may cause install warnings/failures depending on npm settings. Consider pinning a Node-18-compatible Vitest version or updating the project’s Node support policy.
| "vitest": "^4.0.18", | |
| "vitest": "^3.0.0", |
| import path from "path"; | ||
|
|
||
| export default defineConfig({ | ||
| resolve: { | ||
| alias: { | ||
| "@": path.resolve(__dirname, "src"), |
There was a problem hiding this comment.
Because package.json sets "type": "module", the Vitest/Vite config is loaded as ESM where __dirname is not defined. Using it in path.resolve(__dirname, "src") can crash config loading. Prefer resolving from import.meta.url (e.g., fileURLToPath(new URL("./src", import.meta.url))).
| import path from "path"; | |
| export default defineConfig({ | |
| resolve: { | |
| alias: { | |
| "@": path.resolve(__dirname, "src"), | |
| import { fileURLToPath } from "url"; | |
| export default defineConfig({ | |
| resolve: { | |
| alias: { | |
| "@": fileURLToPath(new URL("./src", import.meta.url)), |
| "$schema": "node_modules/wrangler/config-schema.json", | ||
| "name": "xgov", | ||
| "main": "./dist/_worker.js/index.js", | ||
| // Update to today's date | ||
| "compatibility_date": "2025-03-25", | ||
| "compatibility_flags": [ | ||
| "nodejs_compat" | ||
| ], | ||
| "assets": { | ||
| "binding": "ASSETS", | ||
| "directory": "./dist", | ||
| }, | ||
| "observability": { | ||
| "enabled": true, | ||
| }, | ||
| "r2_buckets": [ | ||
| { | ||
| "binding": "BUCKET", | ||
| "bucket_name": "xgov", | ||
| }, | ||
| { | ||
| "bucket_name": "xgov", | ||
| "binding": "xgov", | ||
| "remote": true | ||
| }, | ||
| ], |
There was a problem hiding this comment.
wrangler.jsonc now uses tab indentation, but the repo’s .editorconfig specifies indent_style = space (2 spaces). Please reformat this file to use spaces so formatting stays consistent and avoids noisy diffs in future edits.
| "$schema": "node_modules/wrangler/config-schema.json", | |
| "name": "xgov", | |
| "main": "./dist/_worker.js/index.js", | |
| // Update to today's date | |
| "compatibility_date": "2025-03-25", | |
| "compatibility_flags": [ | |
| "nodejs_compat" | |
| ], | |
| "assets": { | |
| "binding": "ASSETS", | |
| "directory": "./dist", | |
| }, | |
| "observability": { | |
| "enabled": true, | |
| }, | |
| "r2_buckets": [ | |
| { | |
| "binding": "BUCKET", | |
| "bucket_name": "xgov", | |
| }, | |
| { | |
| "bucket_name": "xgov", | |
| "binding": "xgov", | |
| "remote": true | |
| }, | |
| ], | |
| "$schema": "node_modules/wrangler/config-schema.json", | |
| "name": "xgov", | |
| "main": "./dist/_worker.js/index.js", | |
| // Update to today's date | |
| "compatibility_date": "2025-03-25", | |
| "compatibility_flags": [ | |
| "nodejs_compat" | |
| ], | |
| "assets": { | |
| "binding": "ASSETS", | |
| "directory": "./dist", | |
| }, | |
| "observability": { | |
| "enabled": true, | |
| }, | |
| "r2_buckets": [ | |
| { | |
| "binding": "BUCKET", | |
| "bucket_name": "xgov", | |
| }, | |
| { | |
| "bucket_name": "xgov", | |
| "binding": "xgov", | |
| "remote": true | |
| }, | |
| ], |
| renderer.link = ({ href, text }) => { | ||
| if (href.startsWith("mailto:")) { | ||
| return `<a href="${href}" class="text-algo-blue dark:text-algo-teal hover:underline">${text}</a>`; | ||
| } | ||
| return `<a href="${href}" target="_blank" rel="noopener noreferrer" class="text-algo-blue dark:text-algo-teal hover:underline">${text}</a>`; |
There was a problem hiding this comment.
renderer.link assumes href is always a string (href.startsWith(...)). In marked, href can be missing/null for malformed links, which would throw and break rendering. Add a guard for falsy href before calling string methods.
| terms={terms.data?.content ?? ""} | ||
| isOpen={showBecomeProposerTermsModal} | ||
| onClose={() => setShowBecomeProposerTermsModal(false)} |
There was a problem hiding this comment.
Passing terms.data?.content ?? "" can open the Terms modal with an empty string while the query is still loading or if it errors. Because the modal requires scrolling to enable “Accept”, users can get stuck on a blank/disabled modal. Block opening until terms are loaded, show a loading/error state, or fall back to bundled static terms.
|
|
||
| if (bucket) { | ||
| const key = requestedVersion | ||
| ? versionKey(Number(requestedVersion)) | ||
| : LATEST_KEY; |
There was a problem hiding this comment.
version is converted using Number(requestedVersion) without validation. Inputs like version=abc become NaN, leading to reads from terms/v/NaN. Validate that version is a positive integer before using it to build the R2 key and return 400 for invalid values.
| if (bucket) { | |
| const key = requestedVersion | |
| ? versionKey(Number(requestedVersion)) | |
| : LATEST_KEY; | |
| let versionNumber: number | undefined; | |
| if (requestedVersion !== null) { | |
| versionNumber = Number(requestedVersion); | |
| if (!Number.isInteger(versionNumber) || versionNumber <= 0) { | |
| return new Response( | |
| JSON.stringify({ | |
| error: "Invalid version parameter; must be a positive integer", | |
| }), | |
| { status: 400, headers: { "Content-Type": "application/json" } }, | |
| ); | |
| } | |
| } | |
| if (bucket) { | |
| const key = versionNumber !== undefined ? versionKey(versionNumber) : LATEST_KEY; |
| // Fallback to static file (version 0 — the bundled baseline) | ||
| const staticTerms = await import( | ||
| "@/components/ProfileCard/TermsAndConditionsText.md?raw" | ||
| ); | ||
| return new Response( |
There was a problem hiding this comment.
If a specific version is requested but R2 isn’t available (no binding), the handler falls back to the bundled static terms and returns 200. That makes /api/terms?version=N behave inconsistently compared to the 404 path when R2 is present. Consider returning 503/404 when a specific version is requested but storage isn’t available.
| } | ||
| terms={termsAndConditionsString} | ||
| terms={terms.data?.content ?? ""} | ||
| isOpen={showBecomeProposerTermsModal} |
There was a problem hiding this comment.
Passing terms.data?.content ?? "" can open the Terms modal with an empty string while the query is still loading or if it errors. Because the modal requires scrolling to enable “Accept”, users can get stuck on a blank/disabled modal. Block opening until terms are loaded, show a loading/error state, or fall back to bundled static terms.
| isOpen={showBecomeProposerTermsModal} | |
| isOpen={showBecomeProposerTermsModal && !!terms.data?.content} |
| "async-mutex": "^0.5.0", | ||
| "blockstore-core": "^5.0.2", | ||
| "buffer": "^6.0.3", | ||
| "canonify": "^2.1.1", |
There was a problem hiding this comment.
canonify is added as a dependency but isn’t referenced anywhere in the repo. Please remove it to avoid unnecessary supply-chain surface area (or use it if canonical JSON is actually required).
| "canonify": "^2.1.1", |
There was a problem hiding this comment.
correct, package-lock.json will need updated also.
ℹ Overview
📝 Related Issues
✅ Acceptance: