Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughAdds a new CLI package with commands and client, implements a secret-key wrapper for Next.js API routes, introduces a posts API (list/create/get/update/delete) scoped to a page, and updates the Integrations settings UI text to include the CLI. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (chp)
participant Client as ApiClient
participant API as Next.js API (/api/v1/posts)
participant Middleware as withSecretKey
participant DB as Database
CLI->>Client: invoke command (list/get/create/update/delete)
Client->>API: HTTP request (adds header: page-secret-key)
API->>Middleware: handler wrapped by withSecretKey
Middleware->>DB: getPageByIntegrationSecret(secret)
DB-->>Middleware: page record / null
alt page found
Middleware-->>API: call handler with { page }
API->>DB: perform posts query/insert/update/delete
DB-->>API: result
API-->>Client: JSON response (200/201/204)
else invalid secret
Middleware-->>API: return 401 error
API-->>Client: 401 response
end
Client-->>CLI: parsed result or exit on error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/components/page-settings/integrations.tsx (1)
38-38:⚠️ Potential issue | 🟡 MinorPre-existing grammar error in warning dialog copy.
"will be permanently"should be"will be permanent"—permanentlyis an adverb with nothing to modify here.✏️ Proposed fix
-"You will NOT be able to use your old secret and all integrations will have to be updated. This action will be permanently and cannot be undone." +"You will NOT be able to use your old secret and all integrations will have to be updated. This action will be permanent and cannot be undone."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/components/page-settings/integrations.tsx` at line 38, The warning dialog copy contains a grammar mistake: change the phrase "will be permanently" to "will be permanent" in the string literal inside the integrations component (look for the message string in apps/web/components/page-settings/integrations.tsx, e.g., inside the Integrations or PageSettingsIntegrations component where the warning/dialog text is defined) so the sentence reads "This action will be permanent and cannot be undone."
🧹 Nitpick comments (4)
packages/cli/package.json (2)
1-32: Add anenginesfield to declare the minimum Node.js version.Without an
enginesfield, users on older Node versions will install the package and get cryptic runtime errors.Commander 12already requires `Node.js v18 or higher, so at minimum this should be enforced.♻️ Proposed addition
"publishConfig": { "access": "public" - } + }, + "engines": { + "node": ">=18" + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/package.json` around lines 1 - 32, Add an "engines" field to package.json to enforce a minimum Node.js version (Commander v12 requires Node >=18); update the package.json root (the object containing "name", "version", "dependencies") to include an "engines" property such as "node": ">=18" so installs on older Node versions are rejected or warned, preventing cryptic runtime errors when running the CLI (refer to the package.json file and the "commander" dependency to locate where to add this).
17-17:commanderis two major versions behind latest; verify breaking changes before upgrading.The latest stable version of
commanderis14.0.2. The PR pins^12.1.0. Upgrading to v14 introduces breaking changes in v13 (excess positional arguments now error by default, stricter option-flag validation, Help API changes) and requires Node.js v20+ (vs v18+ for v12). Review your current Commander usage for positional arguments and validate that your minimum Node target is v20 before upgrading. Consider also adding anenginesfield to package.json to clarify the minimum Node.js version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/package.json` at line 17, package.json currently pins "commander": "^12.1.0" but v14 is available and v13+ introduce breaking changes; review all CLI code that uses commander (references to Commander API: Command, .argument/.arguments, variadic positional handling, option flag definitions, and help customization) for the stricter positional-argument behavior and option validation, update call signatures to handle excess positional args or set allowExcessArguments behavior, adjust Help API usage if you rely on custom help formatters, and confirm your project Node target (update or add an "engines" field in package.json to require Node >=20 if you decide to upgrade); only bump the dependency to ^14.0.2 after these checks or keep ^12.1.0 if you cannot meet the breaking-change or Node requirements.packages/cli/tsup.config.ts (1)
6-6:dts: trueis unnecessary for a CLI-only binary.Type declarations are only needed when the package is consumed as a library. Since
@changespage/cliis a pure CLI binary, removingdts: truekeeps the build leaner.♻️ Proposed change
export default defineConfig({ entry: ["src/index.ts"], format: ["esm"], - dts: true, clean: true, banner: { js: "#!/usr/bin/env node", }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/tsup.config.ts` at line 6, Remove the unnecessary TypeScript declaration generation by deleting or disabling the dts option in the tsup configuration (the dts: true setting in tsup.config.ts); locate the tsup config export where dts is set and either remove that key or set it to false so the CLI-only package does not emit .d.ts files during build, keeping all other build options intact.packages/cli/src/commands/posts.ts (1)
143-147:deletebypasses theoutput()helper —--prettyis ignoredAll other commands route their output through
output(data, this)which honours the global--prettyflag. Thedeletecommand callsconsole.log(JSON.stringify(...))directly, so--prettyhas no effect on its output, breaking the consistency that callers (scripts, CI pipelines) may rely on.♻️ Proposed fix
- await client.deletePost(id); - console.log(JSON.stringify({ deleted: true, id })); + await client.deletePost(id); + output({ deleted: true, id }, this);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/posts.ts` around lines 143 - 147, The delete command's action handler currently writes its result with console.log(JSON.stringify(...)) so the global --pretty flag is ignored; change the handler to call the existing output helper with the same payload (e.g., output({ deleted: true, id }, this)) after awaiting client.deletePost(id) so output() can apply formatting/--pretty consistently; locate the .action(async function (this: Command, id: string) { ... }) block that uses getClient and client.deletePost and replace the direct console.log call with output(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/pages/api/v1/posts/`[id].ts:
- Around line 42-44: The current branch only sets updates.publication_date when
updates.status === PostStatus.published, so if the status is changed away from
published the old publication_date remains in the DB; update the logic that
handles status changes (the code block referencing updates, PostStatus.published
and publication_date) to explicitly clear publication_date when status is not
published (e.g., set updates.publication_date = null or delete the property so
the update payload removes it) so the database record no longer keeps an old
publication date when a post is reverted to draft.
- Around line 25-29: The current GET handler treats any Supabase error as a 404;
change the error handling after the .single() call so it inspects the Supabase
error code (e.g., error.code === 'PGRST116' or the provider-specific "not found"
indicator) and only returns res.status(404).json({ error: { statusCode: 404,
message: "Post not found" } }) for that not-found code; for all other errors
return a 500 response (res.status(500).json(...)) and log/return the original
error message/details. Update the block that currently checks the variable named
error in the GET handler to implement this branching (and include a brief
server-side log of the unexpected error).
- Around line 67-85: The update call using
supabaseAdmin.from("posts").update(...).eq("id", id).eq("page_id",
page.id).select(POST_SELECT_FIELDS).single() will surface a PGRST116 error when
no row is found, so change the error handling in that block to treat error.code
=== "PGRST116" as a 404: return res.status(404).json({ error: { statusCode: 404,
message: "Post not found" } }); otherwise keep returning 500 with the original
error.message; remove reliance on the subsequent if (!data) guard since
.single() does not yield null for not-found cases.
In `@apps/web/pages/api/v1/posts/index.ts`:
- Around line 72-83: The code currently returns data[0] after the Supabase
insert but does not handle the case where data is null or an empty array; update
the handler around the
supabaseAdmin.from("posts").insert([postData]).select(POST_SELECT_FIELDS) result
to validate that data is a non-empty array (e.g., if (!data || data.length ===
0) return res.status(500).json({ error: { statusCode: 500, message: "Insert
succeeded but no record returned" } })); only then return
res.status(201).json(data[0]); this ensures you avoid a TypeError and return a
clear error when no row is returned.
- Around line 11-18: Query parameters limit and offset are not validated before
being converted to numbers and used in supabaseAdmin.from("posts").range,
allowing NaN or excessively large values; fix by parsing limit and offset with
safe integer parsing (e.g., parseInt), defaulting to 20 and 0 when parsing fails
or yields negative/NaN, clamp limit to a reasonable MAX_LIMIT (e.g., 100) and
ensure offset is non-negative, then compute start = offset and end = offset +
limit - 1 using those sanitized integers before calling .range; update the code
around the const { status, limit = "20", offset = "0" } = req.query and the
query = supabaseAdmin... .range(...) construction to use these validated/clamped
values.
In `@apps/web/utils/withSecretKey.ts`:
- Around line 18-26: The code reads req.headers["page-secret-key"] into
secretKey but doesn't guard against string[]; add an explicit array check
(Array.isArray(secretKey)) and reject arrays (return res.status(401).json({
error: { statusCode: 401, message: "page-secret-key must be a single value" }}))
before converting to String and calling getPageByIntegrationSecret; update the
branch around the secretKey variable and the call to getPageByIntegrationSecret
to only run when secretKey is a single string.
- Around line 35-40: The catch-all in the withSecretKey middleware currently
maps every failure to a 401; update the error handling in the try/catch around
getPageByIntegrationSecret so only authentication failures produce
res.status(401).json(...): if getPageByIntegrationSecret signals an invalid
secret (e.g., returns null/undefined or throws a specific AuthError you already
use), return 401; otherwise treat other exceptions (DB/network/internal) as
server errors by logging full error details via console.error and returning
res.status(500).json({ error: { statusCode: 500, message: "Internal server
error" } }). Ensure you reference the getPageByIntegrationSecret call and the
withSecretKey middleware/res to apply this branching logic, using error.name or
instanceof to distinguish auth errors where appropriate.
In `@packages/cli/README.md`:
- Around line 68-84: Add a new "List options" section to the README documenting
the missing flags used by the list command: include a table rows for `--status
<status>` (allowed: `draft`, `published`, `archived`), `--limit <n>` (Max
results to return, default: 20) and `--offset <n>` (Pagination offset, default:
0); place this section near the existing "Options" / "Create / Update options"
tables so the example that uses `--status` and `--limit` is covered, and ensure
the descriptions match the behavior of the list command flags.
In `@packages/cli/src/client.ts`:
- Around line 29-39: The code currently calls res.json() unconditionally which
throws on non-JSON error bodies; update the logic in client.ts so you only parse
JSON when appropriate: keep the 204 guard, then if res.ok attempt to await
res.json() (or try/catch and fallback to {}), but for non-ok responses avoid
unconditional res.json() — instead try to parse JSON in a try/catch and if
parsing fails use await res.text() (or a default message) to build the error
string, then call console.error and process.exit(1); refer to the response
handling around res.status, res.ok, res.json() and the error
logging/process.exit(1) to locate and change the code.
In `@packages/cli/src/commands/configure.ts`:
- Around line 28-29: Wrap the call to writeConfig in a try/catch to avoid
bubbling filesystem errors to the user (catch errors from writeConfig and call
console.error with a friendly message and the error), and stop hardcoding the
path string by exporting the CONFIG_FILE constant from config.ts and importing
it into configure.ts; then replace the hardcoded message in configure.ts
("Secret key saved to ~/.changes-page/config.json") with a message that uses
CONFIG_FILE so the printed path always reflects the canonical config location.
In `@packages/cli/src/commands/posts.ts`:
- Around line 119-124: The update flow is accidentally overwriting post content
when stdin is redirected but empty because readStdin() returns "" (empty string)
and the current check (if (content !== null)) treats that as present; change the
guard in the posts update code around the body.content assignment to only set
body.content when content is a non-empty string (e.g., use if (content) or if
(content !== null && content !== "") ) so empty piped stdin won't clear the
post; locate the assignment to body.content in posts.ts and adjust the
conditional accordingly (keep readStdin() usage and other fields like
body.title/body.tags unchanged).
- Around line 113-128: The --allow-reactions flag is currently a presence-only
boolean so you cannot set it to false; change the CLI option declaration to
accept an explicit value (e.g., .option("--allow-reactions <value>", "Enable
reactions (true|false)") ) and in the .action handler parse opts.allowReactions
into a boolean (accept "true"/"false" or use a small parseBoolean helper) and
then set body.allow_reactions = parsedBoolean when opts.allowReactions is not
undefined; update the option parsing and the action block around
getClient/opts/content so the new string/boolean value is converted before
assigning to body.allow_reactions.
- Around line 61-62: The code currently uses
Number(opts.limit)/Number(opts.offset) which converts invalid strings to NaN and
causes "limit=NaN" in the query; update the validation in the command handler
that builds the query so that opts.limit and opts.offset are parsed and
validated (e.g., use parseInt or Number and then check Number.isFinite / isNaN)
and either (a) reject invalid input with a clear CLI error message, or (b) omit
the param / fall back to a default when the value is not a finite number; locate
the code around the limit/offset construction (references: opts.limit,
opts.offset, the object that sets limit/offset) and implement the
validation/fallback logic instead of blindly calling Number().
In `@packages/cli/src/config.ts`:
- Around line 31-34: The config file writeConfig currently calls
writeFileSync(CONFIG_FILE, ...) which creates the file with default process
umask (world-readable); change writeConfig to create/write the file with
owner-only permissions (mode 0o600) and ensure the parent directory created by
ensureDir has restrictive permissions; specifically open/write CONFIG_FILE using
an API that accepts a mode (or set file mode after write) so the secret key is
only readable/writable by the owner, and keep references to writeConfig,
ensureDir, CONFIG_FILE and writeFileSync when making the change.
---
Outside diff comments:
In `@apps/web/components/page-settings/integrations.tsx`:
- Line 38: The warning dialog copy contains a grammar mistake: change the phrase
"will be permanently" to "will be permanent" in the string literal inside the
integrations component (look for the message string in
apps/web/components/page-settings/integrations.tsx, e.g., inside the
Integrations or PageSettingsIntegrations component where the warning/dialog text
is defined) so the sentence reads "This action will be permanent and cannot be
undone."
---
Nitpick comments:
In `@packages/cli/package.json`:
- Around line 1-32: Add an "engines" field to package.json to enforce a minimum
Node.js version (Commander v12 requires Node >=18); update the package.json root
(the object containing "name", "version", "dependencies") to include an
"engines" property such as "node": ">=18" so installs on older Node versions are
rejected or warned, preventing cryptic runtime errors when running the CLI
(refer to the package.json file and the "commander" dependency to locate where
to add this).
- Line 17: package.json currently pins "commander": "^12.1.0" but v14 is
available and v13+ introduce breaking changes; review all CLI code that uses
commander (references to Commander API: Command, .argument/.arguments, variadic
positional handling, option flag definitions, and help customization) for the
stricter positional-argument behavior and option validation, update call
signatures to handle excess positional args or set allowExcessArguments
behavior, adjust Help API usage if you rely on custom help formatters, and
confirm your project Node target (update or add an "engines" field in
package.json to require Node >=20 if you decide to upgrade); only bump the
dependency to ^14.0.2 after these checks or keep ^12.1.0 if you cannot meet the
breaking-change or Node requirements.
In `@packages/cli/src/commands/posts.ts`:
- Around line 143-147: The delete command's action handler currently writes its
result with console.log(JSON.stringify(...)) so the global --pretty flag is
ignored; change the handler to call the existing output helper with the same
payload (e.g., output({ deleted: true, id }, this)) after awaiting
client.deletePost(id) so output() can apply formatting/--pretty consistently;
locate the .action(async function (this: Command, id: string) { ... }) block
that uses getClient and client.deletePost and replace the direct console.log
call with output(...).
In `@packages/cli/tsup.config.ts`:
- Line 6: Remove the unnecessary TypeScript declaration generation by deleting
or disabling the dts option in the tsup configuration (the dts: true setting in
tsup.config.ts); locate the tsup config export where dts is set and either
remove that key or set it to false so the CLI-only package does not emit .d.ts
files during build, keeping all other build options intact.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/pages/api/v1/posts/shared.ts (1)
3-6:POST_SELECT_FIELDSstring andIPublicPosttype have no compile-time linkage and can drift.If the Supabase schema gains new non-sensitive columns,
codegenwill expandIPost→IPublicPostautomatically, butPOST_SELECT_FIELDSstays unchanged. The API will be typed as returning those new fields while the DB query omits them.Consider a type-derived constant to keep them in sync:
♻️ Proposed refactor
-export const POST_SELECT_FIELDS = - "id, title, content, tags, status, notes, allow_reactions, publish_at, publication_date, created_at, updated_at"; +// Derive the field list from the type so that adding/removing IPost columns +// forces an explicit update here rather than silently diverging. +const PUBLIC_POST_KEYS: (keyof IPublicPost)[] = [ + "id", "title", "content", "tags", "status", "notes", + "allow_reactions", "publish_at", "publication_date", + "created_at", "updated_at", +]; +export const POST_SELECT_FIELDS = PUBLIC_POST_KEYS.join(", ");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/pages/api/v1/posts/shared.ts` around lines 3 - 6, POST_SELECT_FIELDS is a string literal that can drift away from the IPublicPost type (derived from IPost), causing the API type to include fields the DB query doesn't select; fix by deriving the select fields from the IPublicPost type at compile time (use a utility to get keyof IPublicPost, filter out index signatures/unknowns, and join the keys into a comma-separated string) so POST_SELECT_FIELDS is computed from IPublicPost (and thus IPost) deterministically; update the module to replace the hard-coded POST_SELECT_FIELDS string with this type-derived constant and keep the exported IPublicPost type unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/pages/api/v1/posts/index.ts`:
- Line 36: The handler currently returns Supabase's `data` directly which can be
null; change the response to normalize null to an empty array before sending
(e.g. use `data ?? []` or `Array.isArray(data) ? data : []`) so callers always
receive an array; update the `return res.status(200).json(data);` usage in the
API handler to `res.status(200).json(...)` with the normalized value and keep
existing status/error handling around the `.select()` result.
---
Duplicate comments:
In `@apps/web/pages/api/v1/posts/index.ts`:
- Around line 81-87: The response currently returns
res.status(201).json(data[0]) without guarding that variable; ensure the
insertion result 'data' is validated before accessing index 0. Update the
handler that checks 'error' and 'data' (the same block using error and
res.status) to verify data is non-null and has at least one element; if not,
return a safe error response (e.g., 500 or 204 with an appropriate message)
instead of indexing into data, otherwise return the first item. Use the 'data'
and 'error' variables and the existing res.status(...) flow to implement this
check.
- Around line 10-17: Validate and sanitize limit and offset from req.query
before passing to supabaseAdmin.from(...). Specifically, parse limit and offset
to integers (e.g., Number or parseInt), handle NaN by falling back to the
defaults ("20" and "0"), enforce non-negative offset and a maximum cap for limit
(to prevent abuse), and then pass the validated numeric values into
.range(Number(offset), Number(offset) + Number(limit) - 1) on the posts query;
update the code around req.query, limit, offset and the .range() call to use
these validated/clamped numeric variables.
---
Nitpick comments:
In `@apps/web/pages/api/v1/posts/shared.ts`:
- Around line 3-6: POST_SELECT_FIELDS is a string literal that can drift away
from the IPublicPost type (derived from IPost), causing the API type to include
fields the DB query doesn't select; fix by deriving the select fields from the
IPublicPost type at compile time (use a utility to get keyof IPublicPost, filter
out index signatures/unknowns, and join the keys into a comma-separated string)
so POST_SELECT_FIELDS is computed from IPublicPost (and thus IPost)
deterministically; update the module to replace the hard-coded
POST_SELECT_FIELDS string with this type-derived constant and keep the exported
IPublicPost type unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/client.ts (1)
53-89: Public methods returnPromise<unknown>— consider adding typed overloads.All public methods delegate to
request<T>()without supplyingT, so the inferred return type isPromise<unknown>. Callers inposts.tswill need unsafe casts for any field access. Adding a sharedIPublicPosttype (or co-locating it in the CLI package's shared types) and parameterising each method would give end-to-end safety:♻️ Suggested typed signatures
- async listPosts(params: { - status?: string; - limit?: number; - offset?: number; - }) { + async listPosts(params: { + status?: string; + limit?: number; + offset?: number; + }): Promise<IPublicPost[]> { ... - return this.request(`/posts${qs ? `?${qs}` : ""}`); + return this.request<IPublicPost[]>(`/posts${qs ? `?${qs}` : ""}`); } - async getPost(id: string) { - return this.request(`/posts/${id}`); + async getPost(id: string): Promise<IPublicPost> { + return this.request<IPublicPost>(`/posts/${id}`); } - async createPost(data: Record<string, unknown>) { - return this.request("/posts", { method: "POST", body: JSON.stringify(data) }); + async createPost(data: Record<string, unknown>): Promise<IPublicPost> { + return this.request<IPublicPost>("/posts", { method: "POST", body: JSON.stringify(data) }); } - async updatePost(id: string, data: Record<string, unknown>) { - return this.request(`/posts/${id}`, { method: "PATCH", body: JSON.stringify(data) }); + async updatePost(id: string, data: Record<string, unknown>): Promise<IPublicPost> { + return this.request<IPublicPost>(`/posts/${id}`, { method: "PATCH", body: JSON.stringify(data) }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/client.ts` around lines 53 - 89, The public post methods (listPosts, getPost, createPost, updatePost, deletePost) currently call request() without a type parameter so they all return Promise<unknown>; define or import a shared response type (e.g., IPublicPost and maybe IListPostsResponse) in the CLI package and update each method to call request<T>() with the correct generic (e.g., request<IPublicPost>, request<{ posts: IPublicPost[]; total: number }>, etc.), and adjust method signatures to return those typed promises so callers don’t need unsafe casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/pages/api/v1/posts/`[id].ts:
- Around line 93-107: The DELETE handler currently always returns 204 because
supabaseAdmin.from("posts").delete() doesn't error when no rows match; update
the delete call in the req.method === "DELETE" branch (the block using
supabaseAdmin, .from("posts"), .delete(), .eq("id", id), .eq("page_id",
page.id)) to chain .select("id") so the response includes deleted rows, then
check the returned data (or its length) and if no rows were deleted return
res.status(404).json({ error: { statusCode: 404, message: "Post not found" } })
instead of 204; keep existing error handling for supabase errors.
---
Duplicate comments:
In `@apps/web/pages/api/v1/posts/`[id].ts:
- Around line 46-48: The code only sets updates.publication_date when
updates.status === PostStatus.published but doesn't clear it when status is
changed away from published; update the PATCH handling (the block that currently
checks updates.status === PostStatus.published) to also clear publication_date
when updates.status !== PostStatus.published — e.g., delete
updates.publication_date or set it to null so a draft/no-longer-published post
does not retain the old publication_date before saving.
In `@apps/web/pages/api/v1/posts/index.ts`:
- Line 39: The handler currently returns Supabase's `data` directly
(res.status(200).json(data)), but Supabase types allow `data` to be null; change
the response to send an empty array when `data` is null (e.g., use a
null-coalescing fallback such as `const rows = data ?? []` and return
`res.status(200).json(rows)`) so callers always receive an array; update the
return in the same handler where `data` is used.
- Around line 84-90: The response uses data[0] without ensuring data is non-null
and non-empty, causing a TypeError when Supabase returns null or an empty array;
update the handler after the if (error) check to validate that data is an array
with at least one element (e.g., check data && data.length > 0) and return an
appropriate error response (status and message) when not, otherwise return
data[0]; reference the variables/express objects used in this diff (data, error,
res) to locate and fix the issue.
- Around line 12-13: The limit calculation allows arbitrarily large values;
introduce a MAX_LIMIT constant and cap the computed limit with Math.min to
prevent full-table scans: after parsing rawLimit (used in the existing const
limit = ...), replace the unbounded value with something like const limit =
Math.min(MAX_LIMIT, Math.max(1, Number(rawLimit) || 20)); ensure MAX_LIMIT is
defined near this handler (choose an appropriate sane default, e.g. 100 or 1000)
and use the same capped limit variable wherever pagination is applied.
---
Nitpick comments:
In `@packages/cli/src/client.ts`:
- Around line 53-89: The public post methods (listPosts, getPost, createPost,
updatePost, deletePost) currently call request() without a type parameter so
they all return Promise<unknown>; define or import a shared response type (e.g.,
IPublicPost and maybe IListPostsResponse) in the CLI package and update each
method to call request<T>() with the correct generic (e.g.,
request<IPublicPost>, request<{ posts: IPublicPost[]; total: number }>, etc.),
and adjust method signatures to return those typed promises so callers don’t
need unsafe casts.
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
New Features
Documentation