Skip to content

Promote test to main - #473

Merged
sweetmantech merged 1 commit into
mainfrom
test
Apr 23, 2026
Merged

Promote test to main#473
sweetmantech merged 1 commit into
mainfrom
test

Conversation

@sweetmantech

@sweetmantechsweetmantech commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Promotes test to main.

Included since last promotion

Both migrations move endpoints behind the new account-scoped auth check (checkAccountAccess) and follow the hardened validate<X>Request + <X>Handler pattern.

Verification

🤖 Generated with Claude Code


Summary by cubic

Promotes test to main, migrating catalogs and social scrape APIs to account-scoped routes with stricter auth, validation, and consistent responses.

  • New Features

    • Added GET /api/accounts/{id}/catalogs with CORS, UUID validation, validateAuthContext + canAccessAccount, and Supabase-backed results via selectAccountCatalogs. Response matches the legacy { status, catalogs }.
    • Migrated social scrape to POST /api/socials/{id}/scrape behind account-scoped auth.
  • Bug Fixes

    • Fixed access check to compare the caller’s account ID to the target path ID (no auth override), preventing false-positive access.
    • Masked internal errors on 500 responses from catalogs handler to avoid leaking exception details.

Written for commit 46a8e06. Summary will update on new commits.

* feat: migrate GET /api/catalogs to GET /api/accounts/{id}/catalogs
Port the `GET /api/catalogs?account_id=...` endpoint off the legacy Express
service (`api.recoupable.com`) into a RESTful nested-plural Next.js route
`GET /api/accounts/{id}/catalogs` with auth + access control.
Changes:
- app/api/accounts/[id]/catalogs/route.ts: new OPTIONS + GET route
delegating to getCatalogsHandler.
- lib/catalog/getCatalogsHandler.ts: thin handler; awaits params, calls
the request validator, returns getCatalogsForAccounts result.
- lib/catalog/validateGetCatalogsRequest.ts: UUID path validation +
validateAuthContext + selectAccounts (404) + checkAccountAccess (403).
Single `getCatalogsParamsSchema` reused by the MCP tool.
- lib/catalog/getCatalogsForAccounts.ts: ported from Recoup-Agent-APIs;
flattens account_catalogs → catalogs rows to { id, name, created_at,
updated_at }.
- lib/accounts/checkAccountAccess.ts: new account-scoped access helper
mirroring checkAccountArtistAccess / checkAccountSocialAccess.
- lib/evals/getCatalogSongsCountExpected.ts: drop legacy `getCatalogs`
HTTP helper; call getCatalogsForAccounts in-process.
- lib/mcp/tools/catalogs/registerGetCatalogsTool.ts: repoint to the new
schema + business function.
- lib/catalog/__tests__/: new handler / validator / business-fn tests.
- Delete lib/catalog/getCatalogs.ts and lib/catalog/validateGetCatalogsQuery.ts.
Response body is byte-identical to the legacy endpoint. Auth is new: the
route requires x-api-key or Authorization: Bearer and rejects
cross-account reads with 403.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(catalog): keep caller id distinct from target in access check
Passing `{ accountId: id }` to validateAuthContext rewrote
authResult.accountId to the target path id, turning the subsequent
checkAccountAccess(authResult.accountId, id) into a self-check that
always returned true. Drop the override so the access gate actually
compares the caller against the target, mirroring
validateDeleteArtistRequest.ts. Add regression tests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(catalog): do not leak raw exception text in 500 responses
Replace the dynamic error.message pass-through with a generic
"Internal server error" string in getCatalogsHandler's 500 body.
Server-side console.error is retained so ops still see the detail.
Test asserts the raw exception message never appears in the response.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* refactor: use canAccessAccount directly, drop wrapper
Remove lib/accounts/checkAccountAccess.ts — it was a one-line wrapper
around canAccessAccount, pure indirection (DRY/KISS), and would have
collided by name with the unrelated lib/auth/checkAccountAccess.ts
(which has richer self/artist/workspace/organization semantics).
validateGetCatalogsRequest now calls canAccessAccount from
@/lib/organizations/canAccessAccount directly. Tests updated to mock
the canonical helper; 403 regression test preserved.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* refactor(catalog): inline supabase helper, drop redundant wrapper
Collapse `getCatalogsForAccounts` into `selectAccountCatalogs`:
- Helper now takes a single accountId and returns flat `CatalogSummary[]`
- Handler/MCP tool/evals call supabase layer directly
- Envelope `{ status, catalogs }` built inline at the REST/MCP boundary
All three callers passed single-element arrays; plural signature was
legacy-shaped indirection. Flatten logic now lives once, in the query
layer where the join-shape quirk originates.
* refactor(catalog): trim validator JSDoc to essentials
* refactor(evals): drop verbose comment in catalog song count
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com>
@vercel

vercelBot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
apiReadyReadyPreviewApr 23, 2026 7:52pm

Request Review

@coderabbitai

Copy link
Copy Markdown

Warning

Rate limit exceeded

@sweetmantech has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 16 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 56 minutes and 16 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7bdd94b6-7717-451c-b077-509c68f69069

📥 Commits

Reviewing files that changed from the base of the PR and between c2bde22 and 46a8e06.

⛔ Files ignored due to path filters (3)
  • lib/catalog/__tests__/getCatalogsHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/catalog/__tests__/validateGetCatalogsRequest.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/evals/getCatalogSongsCountExpected.ts is excluded by !**/evals/** and included by lib/**
📒 Files selected for processing (7)
  • app/api/accounts/[id]/catalogs/route.ts
  • lib/catalog/getCatalogs.ts
  • lib/catalog/getCatalogsHandler.ts
  • lib/catalog/validateGetCatalogsQuery.ts
  • lib/catalog/validateGetCatalogsRequest.ts
  • lib/mcp/tools/catalogs/registerGetCatalogsTool.ts
  • lib/supabase/account_catalogs/selectAccountCatalogs.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 10 files

Confidence score: 3/5

  • There is a concrete user-impact risk in lib/catalog/validateGetCatalogsRequest.ts: using canAccessAccount instead of the broader account-scoped checkAccountAccess pattern may incorrectly deny valid artist/workspace catalog access.
  • app/api/accounts/[id]/catalogs/route.ts is flagged for export structure (OPTIONS and GET top-level exports), which introduces moderate integration/maintainability uncertainty even if behavior may still work.
  • The test-size issue in lib/catalog/__tests__/validateGetCatalogsRequest.test.ts is lower severity and mainly maintainability-focused, so it does not by itself make this unmergeable.
  • Pay close attention to lib/catalog/validateGetCatalogsRequest.ts and app/api/accounts/[id]/catalogs/route.ts - access-control correctness and route-module contract are the main risk areas.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/catalog/__tests__/validateGetCatalogsRequest.test.ts">
<violation number="1" location="lib/catalog/__tests__/validateGetCatalogsRequest.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
Test file exceeds the repository’s 100-line limit.</violation>
</file>
<file name="app/api/accounts/[id]/catalogs/route.ts">
<violation number="1" location="app/api/accounts/[id]/catalogs/route.ts:10">
P2: Custom agent: **Module should export a single primary function whose name matches the filename**
Module exports multiple top-level functions (`OPTIONS` and `GET`) instead of a single primary export matching the filename.</violation>
</file>
<file name="lib/catalog/validateGetCatalogsRequest.ts">
<violation number="1" location="lib/catalog/validateGetCatalogsRequest.ts:55">
P1: This authorization check uses `canAccessAccount`, which is narrower than the account-scoped `checkAccountAccess` pattern and can incorrectly deny valid artist/workspace access.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant API as Catalogs API Route
participant Validator as Request Validator
participant Auth as Auth Context
participant DB as Supabase DB
Note over Client, DB: GET /api/accounts/{id}/catalogs Flow
Client->>API: GET /api/accounts/{id}/catalogs
API->>Validator: NEW: validateGetCatalogsRequest(request, id)
Validator->>Validator: Validate ID is UUID
alt Invalid UUID
Validator-->>API: 400 Bad Request
API-->>Client: 400 Error
end
Validator->>Auth: validateAuthContext(request)
Auth-->>Validator: authResult (caller accountId)
alt Auth failed
Auth-->>API: 401 Unauthorized
API-->>Client: 401 Error
end
Validator->>DB: selectAccounts(targetId)
DB-->>Validator: Account record
alt Account not found
Validator-->>API: 404 Not Found
API-->>Client: 404 Error
end
Validator->>Validator: NEW: canAccessAccount(callerId, targetId)
alt Access Denied
Validator-->>API: 403 Forbidden
API-->>Client: 403 Error
end
Validator-->>API: Validated accountId
API->>DB: CHANGED: selectAccountCatalogs(accountId)
Note right of DB: Query filtered by account_id<br/>Ordered by created_at desc
alt Success
DB-->>API: Catalog list
API-->>Client: 200 OK (status, catalogs)
else DB Error
DB-->>API: Exception
API->>API: Mask internal details
API-->>Client: 500 Internal Server error
end
Note over Client, DB: POST /api/socials/{id}/scrape (similar auth pattern)
Loading

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

);
}

const hasAccess = await canAccessAccount({

@cubic-dev-aicubic-dev-aiBotApr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This authorization check uses canAccessAccount, which is narrower than the account-scoped checkAccountAccess pattern and can incorrectly deny valid artist/workspace access.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/validateGetCatalogsRequest.ts, line 55:
<comment>This authorization check uses `canAccessAccount`, which is narrower than the account-scoped `checkAccountAccess` pattern and can incorrectly deny valid artist/workspace access.</comment>
<file context>
@@ -0,0 +1,67 @@
+ );
+ }
+
+ const hasAccess = await canAccessAccount({
+ currentAccountId: authResult.accountId,
+ targetAccountId: accountId,
</file context>
Fix with Cubic

@@ -0,0 +1,129 @@
import { describe, it, expect, vi, beforeEach } from "vitest";

@cubic-dev-aicubic-dev-aiBotApr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Enforce Clear Code Style and Maintainability Practices

Test file exceeds the repository’s 100-line limit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/__tests__/validateGetCatalogsRequest.test.ts, line 1:
<comment>Test file exceeds the repository’s 100-line limit.</comment>
<file context>
@@ -0,0 +1,129 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+
</file context>
Fix with Cubic

@@ -0,0 +1,29 @@
import { NextRequest, NextResponse } from "next/server";

@cubic-dev-aicubic-dev-aiBotApr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Module should export a single primary function whose name matches the filename

Module exports multiple top-level functions (OPTIONS and GET) instead of a single primary export matching the filename.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/accounts/[id]/catalogs/route.ts, line 10:
<comment>Module exports multiple top-level functions (`OPTIONS` and `GET`) instead of a single primary export matching the filename.</comment>
<file context>
@@ -0,0 +1,29 @@
+ *
+ * @returns A NextResponse with CORS headers.
+ */
+export async function OPTIONS() {
+ return new NextResponse(null, {
+ status: 200,
</file context>
Fix with Cubic

@sweetmantech
sweetmantech merged commit 67bddf6 into mainApr 23, 2026
7 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@sweetmantech@arpitgupta1214