feat: integrate Privy authentication for task deletion#1715
Merged
sweetmantech merged 12 commits intotestfrom May 6, 2026
Merged
feat: integrate Privy authentication for task deletion#1715sweetmantech merged 12 commits intotestfrom
sweetmantech merged 12 commits intotestfrom
Conversation
- Updated `useDeleteScheduledAction` to include Privy authentication, requiring an access token for task deletion. - Modified `deleteTask` function to accept an access token and handle API requests accordingly, improving security and error handling. - Enhanced error messages for better user feedback during deletion failures.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR migrates scheduled action deletion from an unauthenticated local API route to authenticated calls to a backend endpoint. The ChangesAuthenticated Task Deletion Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8420d63e08
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+31
to
+33
| if (!response.ok) { | ||
| const errorText = await response.text(); | ||
| throw new Error(`HTTP ${response.status}: ${errorText}`); |
There was a problem hiding this comment.
This error branch now throws on every non-2xx response, but the previous implementation handled the scheduler's "Schedule not found" response by deleting the task record through the local fallback endpoint. Removing that special case regresses deletion for paused tasks (which are still exposed in the UI), so users can hit a hard failure and the task row remains orphaned instead of being removed. Please keep the auth changes, but reinstate equivalent handling for this specific API error path.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Confidence score: 3/5
- There is a concrete regression risk in
lib/tasks/deleteTask.ts: removing the prior "Schedule not found" fallback can cause delete failures for paused tasks that no longer exist in the external scheduler. hooks/useDeleteScheduledAction.tshas a lower-severity UX issue where the sign-in-specific error is overridden by a generic catch toast, reducing actionable feedback for users.- Given a medium-high severity functional risk (7/10) with solid confidence, this carries some merge risk rather than being a clearly safe merge.
- Pay close attention to
lib/tasks/deleteTask.tsandhooks/useDeleteScheduledAction.ts- deletion fallback behavior and error-to-toast mapping may need adjustment to avoid regressions and unclear user messaging.
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="hooks/useDeleteScheduledAction.ts">
<violation number="1" location="hooks/useDeleteScheduledAction.ts:27">
P2: The new sign-in-specific error is masked by the generic catch toast, so users won’t get the intended actionable message in this hook’s UI feedback path.</violation>
</file>
<file name="lib/tasks/deleteTask.ts">
<violation number="1" location="lib/tasks/deleteTask.ts:33">
P1: The new error handling removed the prior "Schedule not found" fallback, which can regress deletion for paused tasks that are no longer present in the external scheduler.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Introduced a constant for the sign-in error message in `useDeleteScheduledAction` for improved consistency. - Updated error handling in `deleteTask` to check for a "Schedule not found" error and fallback to a local delete endpoint. - Added a test case to verify the fallback behavior when the scheduler indicates that a task is not found.
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/tasks/deleteTask.ts (1)
18-24:⚠️ Potential issue | 🟠 MajorDon’t silently succeed when fallback deletion fails.
deleteTaskFromDatabasedoes not checkresponse.ok, so Line 50 can return success even when/api/scheduled-actions/deletefails.Proposed fix
async function deleteTaskFromDatabase(taskId: string): Promise<void> { - await fetch("/api/scheduled-actions/delete", { + const response = await fetch("/api/scheduled-actions/delete", { method: "DELETE", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ id: taskId }), }); + if (!response.ok) { + const errorText = await response.text(); + throw new Error(`Fallback delete failed (HTTP ${response.status}): ${errorText}`); + } }As per coding guidelines, for
lib/**/*.tsutility functions: “Proper error handling”.Also applies to: 49-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/deleteTask.ts` around lines 18 - 24, The deleteTaskFromDatabase function currently ignores the fetch response and may silently succeed on server errors; update deleteTaskFromDatabase to check the fetch response (response.ok) and if not ok, read any response body or status and throw a descriptive Error (including status and response text if available) so callers receive a failure instead of silent success; ensure the function still returns Promise<void> but rejects on non-OK responses (refer to deleteTaskFromDatabase).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/tasks/deleteTask.ts`:
- Around line 56-59: The JSON error path in deleteTask.ts currently throws a
generic error for any data.status === "error", which skips the special "Schedule
not found" handling used earlier; update the block that examines the parsed
response (the data variable in the deleteTask function) to detect when
data.error === "Schedule not found" (or includes that text) and re-raise or
return the same sentinel/error used elsewhere (i.e., mirror the earlier
"Schedule not found" handling) instead of throwing the generic Error("Unknown
error occurred"); ensure you reference and use the same error type or return
value the rest of the function expects so behavior is consistent.
---
Outside diff comments:
In `@lib/tasks/deleteTask.ts`:
- Around line 18-24: The deleteTaskFromDatabase function currently ignores the
fetch response and may silently succeed on server errors; update
deleteTaskFromDatabase to check the fetch response (response.ok) and if not ok,
read any response body or status and throw a descriptive Error (including status
and response text if available) so callers receive a failure instead of silent
success; ensure the function still returns Promise<void> but rejects on non-OK
responses (refer to deleteTaskFromDatabase).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8a1990f-bd95-4cd5-843f-d3b38ef9407e
⛔ Files ignored due to path filters (2)
lib/tasks/__tests__/deleteTask.errors.test.tsis excluded by!**/*.test.*and included bylib/**lib/tasks/__tests__/deleteTask.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (2)
hooks/useDeleteScheduledAction.tslib/tasks/deleteTask.ts
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
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/tasks/deleteTask.ts">
<violation number="1" location="lib/tasks/deleteTask.ts:19">
P2: The fallback local-delete request ignores non-OK HTTP responses, so deletion failures can be reported as success.</violation>
<violation number="2" location="lib/tasks/deleteTask.ts:48">
P2: Handle `"Schedule not found"` in the JSON `status: "error"` branch as well. Right now the fallback only runs for non-OK HTTP responses, so not-found errors returned in a 200 JSON payload still throw instead of cleaning up the local scheduled action.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Enhanced the `deleteTaskFromDatabase` function to throw an error with detailed HTTP status and message when the delete request fails. - Updated the `deleteTask` function to handle "Schedule not found" errors by attempting a local delete fallback. - Added test cases to verify the new error handling and fallback behavior for task deletion.
| successMessage?: string; | ||
| } | ||
|
|
||
| const SIGN_IN_DELETE_TASKS_MESSAGE = "Please sign in to delete tasks"; |
Collaborator
There was a problem hiding this comment.
KISS principle
- actual: this pr adds a new architecture of allowing unathorized access with a toast on attempted action.
- required: follow the existing architecture to hide any content which requires auth until the session is authenticated.
| /** | ||
| * Delete task record from database when scheduler deletion isn't possible | ||
| */ | ||
| async function deleteTaskFromDatabase(taskId: string): Promise<void> { |
Collaborator
There was a problem hiding this comment.
Why is deleteTaskFromDatabase required? There should be no mention of database in the front end client codebase. All database interactions should occur in the API codebase.
…Dialog and TaskCard - Added Privy authentication to conditionally render delete buttons in TaskDetailsDialogActionButtons and TaskCard components. - Updated useDeleteScheduledAction to check for user authentication before allowing task deletion. - Refactored deleteTask function to improve error handling and ensure proper authentication flow during deletion.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
hooks/useDeleteScheduledAction.ts (1)
25-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid silent no-op when auth/token is unavailable.
Line 25 and Line 30 return without surfacing any error, so delete clicks can fail with no user feedback. Please emit a clear sign-in error (or throw) before returning.
Suggested patch
try { if (!authenticated) { + toast.error("Please sign in to delete tasks."); return; } const accessToken = await getAccessToken(); if (!accessToken) { + toast.error("Please sign in to delete tasks."); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useDeleteScheduledAction.ts` around lines 25 - 32, The early-return auth checks in useDeleteScheduledAction silently no-op when the user is unauthenticated or getAccessToken() returns null; replace those bare returns with explicit error propagation by throwing a clear Error (e.g., "Sign-in required") or invoking the hook's existing error handler / onError callback before returning so callers and UI get a visible failure; update the checks around authenticated and getAccessToken() in useDeleteScheduledAction to surface this error rather than silently returning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx`:
- Around line 104-110: The Delete button is enabled even when canEdit is false
which makes clicks no-ops because handleDelete returns early; update the Button
in TaskDetailsDialogActionButtons (the authenticated block rendering Button with
onClick={handleDelete}) to disable when either isLoading or !canEdit (e.g., set
disabled={isLoading || !canEdit}) so the UI reflects permissions; also consider
adding a tooltip or aria-disabled if you want accessibility feedback for the
disabled state.
In `@lib/tasks/deleteTask.ts`:
- Around line 54-56: The fallback DELETE route (DELETE in
app/api/scheduled-actions/delete/route.ts) currently deletes by id with
supabase.from("scheduled_actions").delete() without auth or ownership checks;
update the handler to validate the requester before deleting: authenticate the
request (e.g., validate session/auth header or use Supabase auth to get the user
id) then fetch the scheduled_actions row by id and verify its owner matches the
authenticated user; only call supabase.delete() if the owner matches, otherwise
return 401/403; ensure the same ownership logic is consistent with
deleteTaskViaScheduledActionsRoute and surface clear error responses on
auth/ownership failures.
---
Duplicate comments:
In `@hooks/useDeleteScheduledAction.ts`:
- Around line 25-32: The early-return auth checks in useDeleteScheduledAction
silently no-op when the user is unauthenticated or getAccessToken() returns
null; replace those bare returns with explicit error propagation by throwing a
clear Error (e.g., "Sign-in required") or invoking the hook's existing error
handler / onError callback before returning so callers and UI get a visible
failure; update the checks around authenticated and getAccessToken() in
useDeleteScheduledAction to surface this error rather than silently returning.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8407d423-6ce9-4dd2-8962-8409c9d62304
⛔ Files ignored due to path filters (1)
lib/tasks/__tests__/deleteTask.errors.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (4)
components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsxcomponents/VercelChat/tools/tasks/TaskCard.tsxhooks/useDeleteScheduledAction.tslib/tasks/deleteTask.ts
There was a problem hiding this comment.
3 issues found across 4 files (changes from recent commits).
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="hooks/useDeleteScheduledAction.ts">
<violation number="1" location="hooks/useDeleteScheduledAction.ts:25">
P2: Do not silently return when authentication/token is missing; this makes `deleteAction` look successful and callers continue post-delete UI flow without deleting anything.</violation>
</file>
<file name="lib/tasks/deleteTask.ts">
<violation number="1" location="lib/tasks/deleteTask.ts:55">
P0: Avoid invoking the legacy `/api/scheduled-actions/delete` fallback from this flow until that endpoint enforces authentication and ownership checks; otherwise task deletion can bypass the Privy-protected path.</violation>
</file>
<file name="components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx">
<violation number="1" location="components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx:34">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
File exceeds the 100-line limit required by Rule 3, hurting readability and single-responsibility.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Replaced inline logic with a custom hook `useTaskDetailsDialogActionButtons` to manage state and actions for task details. - Removed unnecessary props and improved button disable logic based on editing permissions. - Streamlined the component structure for better readability and maintainability.
…alogActionButtons - Converted TaskDetailsDialogActionButtons to a functional component with improved prop handling. - Streamlined the useTaskDetailsDialogActionButtons hook by consolidating error handling and removing unnecessary variables. - Added a constant for the sign-in error message in useDeleteScheduledAction to improve consistency in error feedback. - Improved readability and maintainability of both components through structural changes.
…llback logic - Deleted the scheduled actions delete API route as it is no longer needed. - Removed associated fallback logic in the deleteTask function that handled "Schedule not found" errors. - Updated tests to reflect the removal of the fallback behavior for task deletion.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx (1)
20-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisable Pause/Save when editing is not allowed.
These buttons stay enabled while
canEditis false, but their handlers no-op. Please disable them with!canEditto match permissions in the UI.Suggested patch
<Button variant="outline" onClick={handlePause} - disabled={isLoading} + disabled={isLoading || !canEdit} size="sm" > @@ - <Button onClick={handleSave} disabled={isLoading} size="sm"> + <Button onClick={handleSave} disabled={isLoading || !canEdit} size="sm"> {isLoading ? "Saving..." : "Save"} </Button>Also applies to: 42-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx` around lines 20 - 24, The Pause and Save buttons in TaskDetailsDialogActionButtons are still enabled when canEdit is false even though their handlers are no-ops; update the Button components (the one using onClick={handlePause} and the Save button around lines 42-44) to include canEdit in the disabled logic (e.g., disabled={isLoading || !canEdit}) so buttons reflect permissions and are actually disabled when editing is not allowed.
🧹 Nitpick comments (2)
components/VercelChat/dialogs/tasks/useTaskDetailsDialogActionButtons.ts (1)
31-34: ⚡ Quick winExpose action-specific loading state from the hook.
A single
isLoadingconflates delete/pause/save flows and can drive misleading UI text/state downstream. Returning per-action flags (isSaving,isDeleting,isToggling) will keep button labels and disabled logic precise.Also applies to: 78-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VercelChat/dialogs/tasks/useTaskDetailsDialogActionButtons.ts` around lines 31 - 34, The current implementation combines update/delete loading into one isLoading, which masks per-action state; update the code that calls useUpdateScheduledAction and useDeleteScheduledAction (and any toggle/pause hook used around lines ~78-84) to consume and expose distinct flags (e.g., isSaving / isUpdating, isDeleting, isToggling or isPausing) instead of a single isLoading; change the hook call sites (useUpdateScheduledAction -> destructure { updateAction, isSaving } , useDeleteScheduledAction -> { deleteAction, isDeleting } and any toggle hook to { toggleAction, isToggling }) and then replace uses of the combined isLoading with the appropriate per-action flag for button labels and disabled logic so each button reflects its own loading state.hooks/useDeleteScheduledAction.ts (1)
27-49: ⚡ Quick winUse a typed auth error instead of matching
error.message.Control flow based on exact message text is fragile. A dedicated error class/code will keep this stable and easier to maintain.
Suggested patch
+class SignInRequiredError extends Error { + constructor() { + super(SIGN_IN_DELETE_TASKS_MESSAGE); + this.name = "SignInRequiredError"; + } +} @@ - if (!authenticated) { - throw new Error(SIGN_IN_DELETE_TASKS_MESSAGE); - } + if (!authenticated) throw new SignInRequiredError(); @@ - if (!accessToken) { - throw new Error(SIGN_IN_DELETE_TASKS_MESSAGE); - } + if (!accessToken) throw new SignInRequiredError(); @@ - if ( - error instanceof Error && - error.message === SIGN_IN_DELETE_TASKS_MESSAGE - ) { + if (error instanceof SignInRequiredError) { toast.error(SIGN_IN_DELETE_TASKS_MESSAGE); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useDeleteScheduledAction.ts` around lines 27 - 49, Replace fragile string-based error handling in useDeleteScheduledAction: define a dedicated AuthRequiredError (or reuse a shared AuthError) and throw that instead of new Error(SIGN_IN_DELETE_TASKS_MESSAGE) in the authentication checks (the two places calling throw new Error(...)); then update the catch block in useDeleteScheduledAction to test error instanceof AuthRequiredError (rather than matching error.message) and show SIGN_IN_DELETE_TASKS_MESSAGE via toast.error when caught. Ensure the new error class is exported/imported or declared near useDeleteScheduledAction so deleteTask, getAccessToken, and onSuccess flows remain unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx`:
- Around line 20-24: The Pause and Save buttons in
TaskDetailsDialogActionButtons are still enabled when canEdit is false even
though their handlers are no-ops; update the Button components (the one using
onClick={handlePause} and the Save button around lines 42-44) to include canEdit
in the disabled logic (e.g., disabled={isLoading || !canEdit}) so buttons
reflect permissions and are actually disabled when editing is not allowed.
---
Nitpick comments:
In `@components/VercelChat/dialogs/tasks/useTaskDetailsDialogActionButtons.ts`:
- Around line 31-34: The current implementation combines update/delete loading
into one isLoading, which masks per-action state; update the code that calls
useUpdateScheduledAction and useDeleteScheduledAction (and any toggle/pause hook
used around lines ~78-84) to consume and expose distinct flags (e.g., isSaving /
isUpdating, isDeleting, isToggling or isPausing) instead of a single isLoading;
change the hook call sites (useUpdateScheduledAction -> destructure {
updateAction, isSaving } , useDeleteScheduledAction -> { deleteAction,
isDeleting } and any toggle hook to { toggleAction, isToggling }) and then
replace uses of the combined isLoading with the appropriate per-action flag for
button labels and disabled logic so each button reflects its own loading state.
In `@hooks/useDeleteScheduledAction.ts`:
- Around line 27-49: Replace fragile string-based error handling in
useDeleteScheduledAction: define a dedicated AuthRequiredError (or reuse a
shared AuthError) and throw that instead of new
Error(SIGN_IN_DELETE_TASKS_MESSAGE) in the authentication checks (the two places
calling throw new Error(...)); then update the catch block in
useDeleteScheduledAction to test error instanceof AuthRequiredError (rather than
matching error.message) and show SIGN_IN_DELETE_TASKS_MESSAGE via toast.error
when caught. Ensure the new error class is exported/imported or declared near
useDeleteScheduledAction so deleteTask, getAccessToken, and onSuccess flows
remain unaffected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bd68fd54-10df-4f1f-aa08-bceede03e9a6
⛔ Files ignored due to path filters (1)
lib/tasks/__tests__/deleteTask.errors.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (5)
app/api/scheduled-actions/delete/route.tscomponents/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsxcomponents/VercelChat/dialogs/tasks/useTaskDetailsDialogActionButtons.tshooks/useDeleteScheduledAction.tslib/tasks/deleteTask.ts
💤 Files with no reviewable changes (1)
- app/api/scheduled-actions/delete/route.ts
…henticated attempts - Replaced the sign-in error message constant with a new sentinel for unauthenticated delete attempts. - Improved error handling to throw the new error message when deletion is attempted without authentication. - Enhanced logging and user feedback for deletion failures while maintaining existing functionality.
…m hook - Refactored TaskDetailsDialogActionButtons to directly handle task actions, eliminating the use of the custom hook `useTaskDetailsDialogActionButtons`. - Improved prop handling and error management for task updates and deletions. - Streamlined button rendering logic by removing conditional rendering based on authentication, simplifying the component structure. - Updated the delete action to ensure proper error handling and user feedback during task deletion.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx (2)
35-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
isUpdating(not combinedisLoading) for Save button text.When deletion is in progress, Save currently shows
"Saving...", which is inaccurate status copy.Suggested patch
- <Button onClick={handleSave} disabled={isLoading || !canEdit} size="sm"> - {isLoading ? "Saving..." : "Save"} + <Button onClick={handleSave} disabled={isLoading || !canEdit} size="sm"> + {isUpdating ? "Saving..." : "Save"} </Button>Also applies to: 103-104
🤖 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 `@components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx` at line 35, The Save button currently uses the combined isLoading (const isLoading = isUpdating || isDeleting) for its label so when deleting it shows "Saving..."; change the Save button to derive its label/text from isUpdating only (keep using isLoading for general disabled state if desired). Locate the Save button render in TaskDetailsDialogActionButtons (references to isLoading, isUpdating and the Save button text logic) and update the conditional that sets the label to check isUpdating instead of isLoading; apply the same change to the other instance noted (the similar label logic around lines 103-104).
83-87:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisable Pause/Save when
canEditis false to match handler behavior.Line 38 and Line 62 early-return on
!canEdit, but Pause and Save remain interactive. This is a misleading no-op path.Suggested patch
<Button variant="outline" onClick={handlePause} - disabled={isLoading} + disabled={isLoading || !canEdit} size="sm" > @@ - <Button onClick={handleSave} disabled={isLoading} size="sm"> + <Button onClick={handleSave} disabled={isLoading || !canEdit} size="sm"> {isLoading ? "Saving..." : "Save"} </Button>As per coding guidelines, "Always include accessibility in components: use semantic HTML, ARIA attributes, and keyboard navigation."
Also applies to: 103-104
🤖 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 `@components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx` around lines 83 - 87, The Pause and Save buttons in TaskDetailsDialogActionButtons are still interactive even though their handlers (handlePause, handleSave) early-return when canEdit is false; update the Button props to set disabled={!canEdit || isLoading} for both the Pause and Save buttons (and any duplicate buttons around save/confirm) and add appropriate accessibility attributes like aria-disabled={String(!canEdit)} and include keyboard focus handling via a clear focusable state so the buttons are not tabbable when disabled; target the TaskDetailsDialogActionButtons component and the handlePause/handleSave usages to ensure behavior and ARIA state stay in sync.
🤖 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 `@hooks/useDeleteScheduledAction.ts`:
- Around line 13-14: The code uses the string constant
DELETE_WITHOUT_ACCESS_TOKEN as a sentinel by matching error.message which is
brittle; update useDeleteScheduledAction to throw a dedicated sentinel (either a
custom Error subclass like DeleteWithoutAccessTokenError or a branded object
with a unique Symbol) instead of setting the message, and replace all
comparisons of error.message === DELETE_WITHOUT_ACCESS_TOKEN (and the checks at
the other affected spots) with an instanceof check or direct identity comparison
against that branded sentinel; ensure the sentinel is exported/defined alongside
DELETE_WITHOUT_ACCESS_TOKEN so callers can reliably detect and handle this
specific auth-no-token condition.
---
Outside diff comments:
In `@components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx`:
- Line 35: The Save button currently uses the combined isLoading (const
isLoading = isUpdating || isDeleting) for its label so when deleting it shows
"Saving..."; change the Save button to derive its label/text from isUpdating
only (keep using isLoading for general disabled state if desired). Locate the
Save button render in TaskDetailsDialogActionButtons (references to isLoading,
isUpdating and the Save button text logic) and update the conditional that sets
the label to check isUpdating instead of isLoading; apply the same change to the
other instance noted (the similar label logic around lines 103-104).
- Around line 83-87: The Pause and Save buttons in
TaskDetailsDialogActionButtons are still interactive even though their handlers
(handlePause, handleSave) early-return when canEdit is false; update the Button
props to set disabled={!canEdit || isLoading} for both the Pause and Save
buttons (and any duplicate buttons around save/confirm) and add appropriate
accessibility attributes like aria-disabled={String(!canEdit)} and include
keyboard focus handling via a clear focusable state so the buttons are not
tabbable when disabled; target the TaskDetailsDialogActionButtons component and
the handlePause/handleSave usages to ensure behavior and ARIA state stay in
sync.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 465f12df-8175-4e72-b88b-8d7ff1fafccc
📒 Files selected for processing (2)
components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsxhooks/useDeleteScheduledAction.ts
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
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="components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx">
<violation number="1" location="components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx:5">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
File exceeds the 100-line size limit required by the rule.</violation>
<violation number="2" location="components/VercelChat/dialogs/tasks/TaskDetailsDialogActionButtons.tsx:94">
P2: Delete is no longer auth-gated in this component, so unauthenticated users can see and trigger an action that will fail without an access token.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
…cation and streamline task actions - Refactored TaskDetailsDialogActionButtons to utilize Privy for authentication, conditionally rendering the delete button based on user authentication status. - Consolidated task action handlers into a custom hook, improving code organization and readability. - Enhanced prop handling by importing types directly from the mutation handlers, simplifying the component's interface. - Updated error handling in useDeleteScheduledAction to throw a specific error for unauthenticated delete attempts, improving clarity in error management.
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: This PR modifies authentication for task deletion, refactors handler logic, and changes API endpoints. These are not trivial changes; they involve security-critical code and core business logic that a
… hook - Refactored TaskDetailsDialogActionButtons to directly manage task actions, eliminating the custom hook for improved clarity and maintainability. - Enhanced prop handling by defining the interface directly within the component, simplifying the structure. - Updated task action handlers to provide better error handling and user feedback during task updates and deletions. - Removed legacy code related to Privy authentication, ensuring a more straightforward implementation of task actions.
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Requires human review: Removes fallback logic for 'Schedule not found' errors, which may break deletion for paused tasks. Introduces authentication via Privy in core business logic; high risk of regression. Requires human r
sweetmantech
reviewed
May 6, 2026
Collaborator
There was a problem hiding this comment.
KISS - Why are any changes required to this file for your pull request?
…uttons
The Privy auth integration lives in useDeleteScheduledAction.ts and deleteTask.ts
— this file doesn't need touching. Reverting to test removes formatting noise
(inlined objects/JSX, removed intermediate vars, exported interface) and the
redundant `disabled={isLoading || !canEdit}` (the parent already gates rendering
on canEdit at TaskDetailsDialog.tsx:72).
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Collaborator
Manual end-to-end testing on previewPreview deployment: ScenarioOpened the Result
Reproduce# 1. Open the preview
open https://chat-git-feat-chat-tasks-delete-support-recoup.vercel.app
# 2. Sign in via Privy (email code).
# 3. Navigate to /tasks → Schedules tab → click any task to open the details dialog.
# 4. Open DevTools Network tab, filter for "tasks", click Delete.
# Expected:
# - DELETE https://test-recoup-api.vercel.app/api/tasks
# - Authorization: Bearer <Privy JWT>
# - Body: { "id": "<task-id>" }
# - 200 { "status": "success" }
# - Toast: "Scheduled action deleted successfully"
# - Row removed from the Schedules listNotes for reviewer
|
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
2 participants
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.
useDeleteScheduledActionto include Privy authentication, requiring an access token for task deletion.deleteTaskfunction to accept an access token and handle API requests accordingly, improving security and error handling.Summary by CodeRabbit
Security
Refactor