Repo Tooling#211
Open
farhan-navas wants to merge 4 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds Prettier configuration and repo-wide formatting, a new GitHub Actions CI workflow, package-manager/engine updates and tooling (lint-staged, simple-git-hooks), broad code reformatting across the monorepo, and a small set of functional changes: CSRF Set-Cookie forwarding, CSP/x-nonce propagation in proxy, Fastify adapter config, and Ballpit pointer/touch handler refactors. ChangesFormatting, tooling, and CI
Functional/security changes
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend
participant CSRF as CSRF Route
participant Proxy as Middleware Proxy
participant Backend as Backend
participant SSR as Server/SSR
rect rgba(100,200,150,0.5)
Client->>CSRF: GET /api/csrf-token (credentials: include)
CSRF->>Backend: Forward request
Backend-->>CSRF: 200 + Set-Cookie(s) + JSON token
CSRF-->>Client: 200 + all Set-Cookie(s) + token + Cache-Control: no-store
end
rect rgba(150,150,200,0.5)
Client->>Proxy: Request (passes Request object)
Proxy->>Proxy: generate nonce
Proxy->>Proxy: build CSP including nonce
Proxy->>SSR: forward request with x-nonce + Content-Security-Policy
SSR->>Backend: SSR may contact Backend (headers forwarded)
Backend-->>Proxy: response (may include Set-Cookie)
Proxy-->>Client: NextResponse with CSP header + x-nonce + forwarded Set-Cookie(s)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 71-73: The Go module cache misses because actions/setup-go@v5
looks for go.sum at the repository root; update the actions/setup-go@v5 step to
include the cache-dependency-path input pointing to backend/go.sum (i.e., add
cache-dependency-path: backend/go.sum alongside go-version) so the action can
find and use the correct go.sum for caching.
- Around line 84-88: The CI job currently runs the "go vet" and "go build" steps
but does not run tests; add a new step (e.g., name: "go test") between or after
the existing "go vet" and "go build" steps that executes "go test ./..." (or "go
test ./... -v") so unit tests across the repository (including backend/) are
executed and failures break the workflow; reference the existing step names "go
vet" and "go build" to locate where to insert the new "go test" step.
In `@frontend/app/api/csrf-token/route.ts`:
- Around line 5-9: Remove the unnecessary "Content-Type": "application/json"
header from the fetch call in the csrf-token route (the fetch invocation that
calls `${env.BACKEND_URL}/csrf-token` with method "GET"); either delete that
header entry entirely for GET requests or only add Content-Type when the method
is not GET (e.g., in the code paths around the fetch call), so the GET request
no longer carries a misleading Content-Type header.
In `@frontend/components/Ballpit.tsx`:
- Around line 682-692: The pointer update currently overwrites
position/nPosition via updatePointerData before checking isInside(rect), causing
data.onMove to be called with out-of-bounds coordinates; preserve the last
in-bounds coordinates (e.g., add data.lastInBoundsPosition or capture previous
position before updatePointerData) and update that when inside the rect (in the
isInside branch). Then, in the else if (data.hover && data.touching) branch,
call data.onMove with the preserved last-in-bounds coordinates instead of the
current out-of-bounds position; ensure updatePointerData still runs but use the
stored valid point for onMove and update data.lastInBoundsPosition when entering
or moving inside.
- Around line 725-735: The touch handlers (onTouchStart and onTouchMove)
currently call e.preventDefault() unconditionally while being added to
document.body, which blocks page gestures site-wide; update the event wiring so
touch events are attached to the canvas element instead (use the canvas ref /
element instead of document.body when calling addEventListener for "touchstart",
"touchmove", "touchend", "touchcancel" with { passive: false }), or
alternatively modify onTouchStart/onTouchMove to first check isInside(rect) and
only call e.preventDefault() when the touch point is inside the canvas bounds;
make this change for onTouchStart, onTouchMove (and ensure touches
ending/canceling remain scoped to the canvas) so default page touch behaviors
are not prevented outside the canvas.
In `@frontend/content/docs/express.mdx`:
- Around line 15-18: The Tab components currently contain inline triple-backtick
strings which can break MDX rendering; update each <Tab> (values "npm", "yarn",
"pnpm", "bun") so the installation commands are wrapped as proper block-fenced
code blocks: open a fenced block with ```bash on its own line, put the
single-line install command on the next line, then close the fence, and place
that fenced block inside the <Tab> children (rather than a single inline
string). Ensure each Tab (the Tab component instances in this file) follows this
pattern and preserves the same command text for each package manager.
In `@package.json`:
- Around line 1-25: Add a short note to the project README or CONTRIBUTING.md
explaining that the prepare lifecycle script ("prepare": "simple-git-hooks")
runs during pnpm install and must be executed once by contributors after cloning
to activate the simple-git-hooks pre-commit hook (which invokes lint-staged);
mention that running pnpm install at the repo root will install devDependencies
and trigger the prepare hook so pre-commit formatting via prettier (configured
in lint-staged) works as expected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 335f742e-ad87-440e-958d-0b950f946d7f
⛔ Files ignored due to path filters (11)
dbSchema/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlmythos/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlplayground/merchant-express/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlplayground/merchant-fastify/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlplayground/merchant-hono/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlplayground/merchant-nextjs/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlscripts/test-transaction/package-lock.jsonis excluded by!**/package-lock.jsonscripts/test-transaction/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsdk/sdk-typescript/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (110)
.github/workflows/ci.yml.prettierignore.prettierrc.jsonCLAUDE.mdREADME.mdbackend/sangria-backenddbSchema/CLAUDE.mddbSchema/README.mddbSchema/package.jsondbSchema/schema.tsdeployment/DEPLOYMENT.mdfrontend/.prettierrcfrontend/README.mdfrontend/app/(marketing)/blog/[slug]/page.tsxfrontend/app/(marketing)/blog/page.tsxfrontend/app/(marketing)/docs/[[...slug]]/page.tsxfrontend/app/(marketing)/docs/docs-layout-client.tsxfrontend/app/(marketing)/docs/layout.tsxfrontend/app/(marketing)/page.tsxfrontend/app/(portal)/dashboard/api-keys/APIKeysContent.tsxfrontend/app/(portal)/dashboard/api-keys/page.tsxfrontend/app/(portal)/dashboard/members/OrganizationMembersContent.tsxfrontend/app/(portal)/dashboard/members/page.tsxfrontend/app/(portal)/dashboard/organizations/page.tsxfrontend/app/(portal)/dashboard/transactions/TransactionsContent.tsxfrontend/app/(portal)/dashboard/withdrawals/WithdrawModal.tsxfrontend/app/(portal)/dashboard/withdrawals/WithdrawalsContent.tsxfrontend/app/(portal)/layout.tsxfrontend/app/accept-invitation/page.tsxfrontend/app/api/csrf-token/route.tsfrontend/app/globals.cssfrontend/app/layout.tsxfrontend/components/ArcadeButton.tsxfrontend/components/Ballpit.tsxfrontend/components/BlogHeader.tsxfrontend/components/Footer.tsxfrontend/components/GitHubStarChip.tsxfrontend/components/MdxComponents.tsxfrontend/components/NavLinks.tsxfrontend/components/Navigation.tsxfrontend/components/OrganizationDropdown.tsxfrontend/components/PortalSidebarNav.tsxfrontend/components/ProfilePopover.tsxfrontend/components/ResizableSidebar.tsxfrontend/components/ScrollNav.tsxfrontend/components/SignOutButton.tsxfrontend/components/ThemeProvider.tsxfrontend/components/ThemeToggle.tsxfrontend/content/docs/express.mdxfrontend/content/docs/fastapi.mdxfrontend/content/docs/fastify.mdxfrontend/content/docs/hono.mdxfrontend/content/docs/index.mdxfrontend/content/docs/nextjs.mdxfrontend/contexts/OrganizationContext.tsxfrontend/lib/api-proxy.tsfrontend/lib/blog.tsfrontend/lib/fetch.tsfrontend/lib/security-hooks.tsfrontend/lib/security.tsfrontend/lib/validation.tsfrontend/next.config.tsfrontend/package.jsonfrontend/proxy.tsmythos/AGENTS.mdmythos/README.mdmythos/app/(admin)/NavLinks.tsxmythos/app/(admin)/dashboard/page.tsxmythos/app/(admin)/newspaper/page.tsxmythos/app/(admin)/transactions/TransactionsContent.tsxmythos/app/(admin)/withdrawals/ActionDialog.tsxmythos/app/(admin)/withdrawals/AdminWithdrawalsContent.tsxmythos/app/access-denied/page.tsxmythos/app/api/admin/transactions/[id]/ledger/route.tsmythos/app/api/admin/withdrawals/[id]/approve/route.tsmythos/app/api/admin/withdrawals/[id]/complete/route.tsmythos/app/api/admin/withdrawals/[id]/fail/route.tsmythos/app/api/admin/withdrawals/[id]/reject/route.tsmythos/app/api/admin/withdrawals/route.tsmythos/app/api/x402-pay/route.tsmythos/lib/api-proxy.tsmythos/package.jsonpackage.jsonplayground/README.mdplayground/merchant-express/package.jsonplayground/merchant-express/src/index.tsplayground/merchant-fastify/package.jsonplayground/merchant-fastify/src/index.tsplayground/merchant-hono/package.jsonplayground/merchant-hono/src/index.tsplayground/merchant-nextjs/app/api/premium/route.tsplayground/merchant-nextjs/app/layout.tsxplayground/merchant-nextjs/package.jsonplayground/merchant-nextjs/tsconfig.jsonplayground/merchant-nextjs/tsconfig.tsbuildinfoscripts/test-transaction/package.jsonscripts/test-transaction/test-transaction.tssdk/README.mdsdk/sdk-typescript/README.mdsdk/sdk-typescript/package.jsonsdk/sdk-typescript/src/adapters/express.tssdk/sdk-typescript/src/adapters/fastify.tssdk/sdk-typescript/src/adapters/hono.tssdk/sdk-typescript/src/adapters/nextjs.tssdk/sdk-typescript/src/core.tssdk/sdk-typescript/src/index.tssdk/sdk-typescript/src/types.tssdk/sdk-typescript/test-server/server-fastify.tssdk/sdk-typescript/test-server/server-hono.tssdk/sdk-typescript/test-server/server.ts
💤 Files with no reviewable changes (2)
- frontend/.prettierrc
- frontend/app/globals.css
Comment on lines
+71
to
+73
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: "1.25" |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Go module cache will miss — cache-dependency-path needs to point to backend/go.sum
actions/setup-go@v5 is a uses: step and is not subject to defaults.run.working-directory. It searches for go.sum at the repository root; backend/go.sum is invisible to it. Every run will cache-miss and re-download modules.
⚡ Proposed fix
- uses: actions/setup-go@v5
with:
go-version: "1.25"
+ cache-dependency-path: backend/go.sum🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 71 - 73, The Go module cache misses
because actions/setup-go@v5 looks for go.sum at the repository root; update the
actions/setup-go@v5 step to include the cache-dependency-path input pointing to
backend/go.sum (i.e., add cache-dependency-path: backend/go.sum alongside
go-version) so the action can find and use the correct go.sum for caching.
Comment on lines
+84
to
+88
| - name: go vet | ||
| run: go vet ./... | ||
|
|
||
| - name: go build | ||
| run: go build ./... |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Go CI job runs no tests — go test ./... is absent
The job only verifies formatting, vet, and that the binary compiles. Test regressions in backend/ won't be caught until deployment.
🧪 Proposed addition
- name: go build
run: go build ./...
+
+ - name: go test
+ run: go test ./...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: go vet | |
| run: go vet ./... | |
| - name: go build | |
| run: go build ./... | |
| - name: go vet | |
| run: go vet ./... | |
| - name: go build | |
| run: go build ./... | |
| - name: go test | |
| run: go test ./... |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 84 - 88, The CI job currently runs the
"go vet" and "go build" steps but does not run tests; add a new step (e.g.,
name: "go test") between or after the existing "go vet" and "go build" steps
that executes "go test ./..." (or "go test ./... -v") so unit tests across the
repository (including backend/) are executed and failures break the workflow;
reference the existing step names "go vet" and "go build" to locate where to
insert the new "go test" step.
Comment on lines
5
to
9
| const response = await fetch(`${env.BACKEND_URL}/csrf-token`, { | ||
| method: 'GET', | ||
| method: "GET", | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| "Content-Type": "application/json", | ||
| }, |
Contributor
There was a problem hiding this comment.
Content-Type: application/json on a GET request is semantically unnecessary.
GET requests carry no body, so the Content-Type header has no meaning here and is likely carried over from a copy-paste. It's harmless but can be confusing.
🛠️ Suggested fix
const response = await fetch(`${env.BACKEND_URL}/csrf-token`, {
- method: "GET",
- headers: {
- "Content-Type": "application/json",
- },
+ method: "GET",
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/api/csrf-token/route.ts` around lines 5 - 9, Remove the
unnecessary "Content-Type": "application/json" header from the fetch call in the
csrf-token route (the fetch invocation that calls
`${env.BACKEND_URL}/csrf-token` with method "GET"); either delete that header
entry entirely for GET requests or only add Content-Type when the method is not
GET (e.g., in the code paths around the fetch call), so the GET request no
longer carries a misleading Content-Type header.
Comment on lines
682
to
+692
| updatePointerData(data, rect); | ||
| if (isInside(rect)) { if (!data.hover) { data.hover = true; data.touching = true; data.onEnter(data); } data.onMove(data); } | ||
| else if (data.hover && data.touching) { data.onMove(data); } | ||
| if (isInside(rect)) { | ||
| if (!data.hover) { | ||
| data.hover = true; | ||
| data.touching = true; | ||
| data.onEnter(data); | ||
| } | ||
| data.onMove(data); | ||
| } else if (data.hover && data.touching) { | ||
| data.onMove(data); | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve the last in-bounds touch position when dragging outside.
Line 682 updates position/nPosition before the isInside(rect) check. The else if (data.hover && data.touching) path then keeps calling onMove with out-of-bounds coordinates instead of the last valid in-canvas point, which makes the control sphere jump as soon as the finger leaves the element.
💡 Minimal fix
for (const [elem, data] of pointerMap) {
const rect = elem.getBoundingClientRect();
- updatePointerData(data, rect);
if (isInside(rect)) {
+ updatePointerData(data, rect);
if (!data.hover) {
data.hover = true;
data.touching = true;
data.onEnter(data);
}
data.onMove(data);
} else if (data.hover && data.touching) {
data.onMove(data);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| updatePointerData(data, rect); | |
| if (isInside(rect)) { if (!data.hover) { data.hover = true; data.touching = true; data.onEnter(data); } data.onMove(data); } | |
| else if (data.hover && data.touching) { data.onMove(data); } | |
| if (isInside(rect)) { | |
| if (!data.hover) { | |
| data.hover = true; | |
| data.touching = true; | |
| data.onEnter(data); | |
| } | |
| data.onMove(data); | |
| } else if (data.hover && data.touching) { | |
| data.onMove(data); | |
| } | |
| if (isInside(rect)) { | |
| updatePointerData(data, rect); | |
| if (!data.hover) { | |
| data.hover = true; | |
| data.touching = true; | |
| data.onEnter(data); | |
| } | |
| data.onMove(data); | |
| } else if (data.hover && data.touching) { | |
| data.onMove(data); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/Ballpit.tsx` around lines 682 - 692, The pointer update
currently overwrites position/nPosition via updatePointerData before checking
isInside(rect), causing data.onMove to be called with out-of-bounds coordinates;
preserve the last in-bounds coordinates (e.g., add data.lastInBoundsPosition or
capture previous position before updatePointerData) and update that when inside
the rect (in the isInside branch). Then, in the else if (data.hover &&
data.touching) branch, call data.onMove with the preserved last-in-bounds
coordinates instead of the current out-of-bounds position; ensure
updatePointerData still runs but use the stored valid point for onMove and
update data.lastInBoundsPosition when entering or moving inside.
Comment on lines
+725
to
+735
| document.body.addEventListener("pointermove", onPointerMove as EventListener); | ||
| document.body.addEventListener("pointerleave", onPointerLeave as EventListener); | ||
| document.body.addEventListener("click", onPointerClick as EventListener); | ||
| document.body.addEventListener("touchstart", onTouchStart as EventListener, { | ||
| passive: false, | ||
| }); | ||
| document.body.addEventListener("touchmove", onTouchMove as EventListener, { passive: false }); | ||
| document.body.addEventListener("touchend", onTouchEnd as EventListener, { passive: false }); | ||
| document.body.addEventListener("touchcancel", onTouchEnd as EventListener, { | ||
| passive: false, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "Ballpit\.tsx$"Repository: GTG-Labs/sangria
Length of output: 92
🏁 Script executed:
wc -l frontend/components/Ballpit.tsxRepository: GTG-Labs/sangria
Length of output: 96
🏁 Script executed:
sed -n '700,760p' frontend/components/Ballpit.tsxRepository: GTG-Labs/sangria
Length of output: 2292
🏁 Script executed:
sed -n '1,724p' frontend/components/Ballpit.tsx | grep -n "onTouchStart\|onTouchMove\|onPointerMove" | head -20Repository: GTG-Labs/sangria
Length of output: 191
🏁 Script executed:
sed -n '638,698p' frontend/components/Ballpit.tsxRepository: GTG-Labs/sangria
Length of output: 1760
🏁 Script executed:
sed -n '620,640p' frontend/components/Ballpit.tsxRepository: GTG-Labs/sangria
Length of output: 596
🏁 Script executed:
sed -n '600,625p' frontend/components/Ballpit.tsxRepository: GTG-Labs/sangria
Length of output: 805
Move the touch listeners to the canvas element or call preventDefault() only after confirming the touch is within bounds.
The onTouchStart and onTouchMove handlers call e.preventDefault() unconditionally on lines 663 and 681, before checking isInside(rect). Since these non-passive listeners are registered on document.body, this prevents default touch behaviors (scrolling, pinch-zoom) for every touch on the page while any Ballpit component is mounted. Scope the listeners to the canvas element, or check bounds before calling preventDefault().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/Ballpit.tsx` around lines 725 - 735, The touch handlers
(onTouchStart and onTouchMove) currently call e.preventDefault() unconditionally
while being added to document.body, which blocks page gestures site-wide; update
the event wiring so touch events are attached to the canvas element instead (use
the canvas ref / element instead of document.body when calling addEventListener
for "touchstart", "touchmove", "touchend", "touchcancel" with { passive: false
}), or alternatively modify onTouchStart/onTouchMove to first check
isInside(rect) and only call e.preventDefault() when the touch point is inside
the canvas bounds; make this change for onTouchStart, onTouchMove (and ensure
touches ending/canceling remain scoped to the canvas) so default page touch
behaviors are not prevented outside the canvas.
Comment on lines
+15
to
+18
| <Tab value="npm">```bash npm install @sangria-sdk/core ```</Tab> | ||
| <Tab value="yarn">```bash yarn add @sangria-sdk/core ```</Tab> | ||
| <Tab value="pnpm">```bash pnpm add @sangria-sdk/core ```</Tab> | ||
| <Tab value="bun">```bash bun add @sangria-sdk/core ```</Tab> |
Contributor
There was a problem hiding this comment.
Fix tabbed install snippets: inline code fences can break MDX rendering.
These should be block fenced code blocks inside each <Tab>, not inline triple-backtick strings.
📄 Suggested docs fix
- <Tab value="npm">```bash npm install `@sangria-sdk/core` ```</Tab>
- <Tab value="yarn">```bash yarn add `@sangria-sdk/core` ```</Tab>
- <Tab value="pnpm">```bash pnpm add `@sangria-sdk/core` ```</Tab>
- <Tab value="bun">```bash bun add `@sangria-sdk/core` ```</Tab>
+ <Tab value="npm">
+ ```bash
+ npm install `@sangria-sdk/core`
+ ```
+ </Tab>
+ <Tab value="yarn">
+ ```bash
+ yarn add `@sangria-sdk/core`
+ ```
+ </Tab>
+ <Tab value="pnpm">
+ ```bash
+ pnpm add `@sangria-sdk/core`
+ ```
+ </Tab>
+ <Tab value="bun">
+ ```bash
+ bun add `@sangria-sdk/core`
+ ```
+ </Tab>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Tab value="npm">```bash npm install @sangria-sdk/core ```</Tab> | |
| <Tab value="yarn">```bash yarn add @sangria-sdk/core ```</Tab> | |
| <Tab value="pnpm">```bash pnpm add @sangria-sdk/core ```</Tab> | |
| <Tab value="bun">```bash bun add @sangria-sdk/core ```</Tab> | |
| <Tab value="npm"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/content/docs/express.mdx` around lines 15 - 18, The Tab components
currently contain inline triple-backtick strings which can break MDX rendering;
update each <Tab> (values "npm", "yarn", "pnpm", "bun") so the installation
commands are wrapped as proper block-fenced code blocks: open a fenced block
with ```bash on its own line, put the single-line install command on the next
line, then close the fence, and place that fenced block inside the <Tab>
children (rather than a single inline string). Ensure each Tab (the Tab
component instances in this file) follows this pattern and preserves the same
command text for each package manager.
Comment on lines
+1
to
+25
| { | ||
| "name": "sangria-net-tooling", | ||
| "private": true, | ||
| "version": "0.0.0", | ||
| "description": "Root tooling only (pre-commit hooks). Each app/package has its own package.json and lockfile — this repo is NOT a pnpm workspace.", | ||
| "packageManager": "pnpm@10.15.1", | ||
| "engines": { | ||
| "node": ">=20", | ||
| "pnpm": ">=10" | ||
| }, | ||
| "scripts": { | ||
| "prepare": "simple-git-hooks" | ||
| }, | ||
| "devDependencies": { | ||
| "lint-staged": "^15.2.10", | ||
| "prettier": "^3.8.1", | ||
| "simple-git-hooks": "^2.11.1" | ||
| }, | ||
| "simple-git-hooks": { | ||
| "pre-commit": "pnpm exec lint-staged" | ||
| }, | ||
| "lint-staged": { | ||
| "*.{ts,tsx,js,jsx,json,md,mdx,yaml,yml,css}": "prettier --write" | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Solid pre-commit hook setup.
simple-git-hooks + lint-staged is a lightweight and correct pattern here. A few notes:
- The
lint-stagedprettier command omits--ignore-path, which is intentional and correct: unlike sub-packages that reference../.prettierignore, the root config runs from the same directory as.prettierignore, so prettier auto-discovers it. - The
preparelifecycle script must execute for hooks to activate. Contributors need to runpnpm installat the repo root once after cloning; consider documenting this in the project's contributing guide orREADME.mdif not already covered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 1 - 25, Add a short note to the project README or
CONTRIBUTING.md explaining that the prepare lifecycle script ("prepare":
"simple-git-hooks") runs during pnpm install and must be executed once by
contributors after cloning to activate the simple-git-hooks pre-commit hook
(which invokes lint-staged); mention that running pnpm install at the repo root
will install devDependencies and trigger the prepare hook so pre-commit
formatting via prettier (configured in lint-staged) works as expected.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
85-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd Go tests to CI.
As noted in a previous review, the Go job verifies formatting, vetting, and compilation but does not run tests. This means test regressions in
backend/won't be caught until deployment.🧪 Proposed addition
- name: go vet run: go vet ./... + - name: go test + run: go test ./... + - name: go build run: go build ./...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 85 - 89, The CI currently runs "go vet" and "go build" but does not execute unit tests; add a new step after the "go build" step to run the Go test suite (e.g., execute "go test ./..." or "go test ./... -v") so tests in packages like backend/ are exercised during CI; update the workflow steps named "go vet" / "go build" to include a subsequent "go test" step to fail the job on test failures and surface regressions early.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 85-89: The CI currently runs "go vet" and "go build" but does not
execute unit tests; add a new step after the "go build" step to run the Go test
suite (e.g., execute "go test ./..." or "go test ./... -v") so tests in packages
like backend/ are exercised during CI; update the workflow steps named "go vet"
/ "go build" to include a subsequent "go test" step to fail the job on test
failures and surface regressions early.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f00207c5-243b-4f06-8097-80e8bf7e7408
📒 Files selected for processing (2)
.github/workflows/ci.ymlpackage.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Bug Fixes
Documentation