Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughReplaces legacy workflow-runs modules with action-based APIs/hooks (ActionRun and action logs), removes workflow-runs query/keys, updates hooks and UI to use ActionRun shapes and keys, converts several components to named exports, and standardizes the DiffEditor API and usage. (34 words) Changes
Sequence Diagram(s)mermaid Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
| // interface WorkflowRun { | ||
| // id: string | ||
| // status: string | ||
| // branch_id?: string | ||
| // check_run_id?: number | null | ||
| // created_at?: string | ||
| // updated_at?: string | ||
| // workdir?: string | null | ||
| // git_config?: unknown | ||
| // } |
There was a problem hiding this comment.
should be obtained from API generated types, now exported in workflow-run-query
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @apps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsx:
- Around line 50-54: The failure detection is inconsistent and empty run_steps
makes .every() return true; update the boolean logic for showSuccessIcon,
isFailed and isPolling to match workflow-run-query.ts: use .some() for isFailed
(so any DEAD marks failure), keep isPolling logic but guard all checks with a
run_steps length check (e.g., workflowRun?.run_steps?.length > 0) to avoid true
on empty arrays, and ensure showSuccessIcon requires length > 0 and all steps
EXITED; adjust the expressions named showSuccessIcon, isFailed, and isPolling
accordingly.
In @apps/studio/data/workflow-runs/workflow-run-query.ts:
- Around line 35-39: The status computation uses data?.run_steps
(isSuccess/isFailed) before checking for an API error, which can yield undefined
results; move the error handling so that the error is checked and handled
(handleError(error)) before computing isSuccess and isFailed, then compute
status from data.run_steps (or return early if data is undefined) and finally
return the WorkflowRun object; update references in this block including data,
error, isSuccess, isFailed, run_steps and the final return to reflect the
reordered/guarded logic.
In @apps/studio/pages/project/[ref]/merge.tsx:
- Around line 161-163: The isWorkflowRunning check only inspects
currentBranchWorkflow; update its definition to account for both workflows by
using the combined currentWorkflowRun status or explicitly checking both branch
workflows (e.g., replace currentBranchWorkflow?.status === 'RUNNING' with
currentWorkflowRun?.status === 'RUNNING' or currentBranchWorkflow?.status ===
'RUNNING' || parentBranchWorkflow?.status === 'RUNNING') so the merge button is
disabled when either workflow is running.
🧹 Nitpick comments (2)
apps/studio/hooks/branches/useWorkflowManagement.ts (1)
29-36: Missingenabledcondition on logs query.
useWorkflowRunLogsQuerylacks anenabledcondition, unlikeuseWorkflowRunQueryon line 22. While the underlying query has its own enabled check, adding an explicitenabled: Boolean(workflowRunId)here maintains consistency and makes the intent clearer.Suggested fix
const { data: workflowRunLogs } = useWorkflowRunLogsQuery( { projectRef, workflowRunId }, { + enabled: Boolean(workflowRunId), refetchInterval: 2000, refetchOnMount: 'always', staleTime: 0, } )apps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsx (1)
8-17: Remove commented-out code.Dead code should be removed rather than commented out. Version control preserves history if needed later.
Remove commented code
-// interface WorkflowRun { -// id: string -// status: string -// branch_id?: string -// check_run_id?: number | null -// created_at?: string -// updated_at?: string -// workdir?: string | null -// git_config?: unknown -// } -
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsxapps/studio/data/workflow-runs/keys.tsapps/studio/data/workflow-runs/workflow-run-logs-query.tsapps/studio/data/workflow-runs/workflow-run-query.tsapps/studio/data/workflow-runs/workflow-runs-query.tsapps/studio/hooks/branches/useWorkflowManagement.tsapps/studio/pages/project/[ref]/merge.tsx
💤 Files with no reviewable changes (1)
- apps/studio/data/workflow-runs/workflow-runs-query.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/studio/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/studio-best-practices.mdc)
apps/studio/**/*.{ts,tsx}: Assign complex conditions to descriptive variables instead of using multiple conditions in a single expression
Use consistent naming conventions for boolean variables:isprefix for state/identity,hasprefix for possession,canprefix for capability/permission,shouldprefix for conditional behavior
Derive boolean state from existing state instead of storing it separately
Use early returns for guard clauses instead of deeply nested conditionals
Extract complex logic into custom hooks when logic becomes reusable or complex
Return objects from custom hooks instead of arrays for better extensibility and clearer API
Use discriminated unions for complex state management instead of multiple independent state fields
Avoid type casting (e.g.,as any,as Type); instead validate values at runtime using zod schemas
Files:
apps/studio/data/workflow-runs/keys.tsapps/studio/hooks/branches/useWorkflowManagement.tsapps/studio/pages/project/[ref]/merge.tsxapps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsxapps/studio/data/workflow-runs/workflow-run-logs-query.tsapps/studio/data/workflow-runs/workflow-run-query.ts
apps/studio/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/studio-best-practices.mdc)
apps/studio/**/*.tsx: Components should ideally be under 200-300 lines; break down large components with multiple distinct UI sections, complex conditional rendering, or multiple unrelated useState hooks
Extract repeated JSX patterns into reusable components instead of copying similar JSX blocks
Use consistent loading/error/success pattern: handle loading state first with early returns, then error state, then empty state, then render success state
Keep state as local as possible and only lift up when needed
Group related state using objects or reducers instead of multiple useState calls, preferring react-hook-form for form state management
Name event handlers consistently: useonprefix for prop callbacks andhandleprefix for internal handlers
Avoid inline arrow functions for expensive operations; use useCallback to maintain stable function references
Use appropriate conditional rendering patterns: && for simple show/hide, ternary for binary choice, early returns or extracted components for multiple conditions
Avoid nested ternaries in JSX; use separate conditions or early returns instead
Use useMemo for expensive computations when the computation is genuinely expensive and the value is passed to memoized children
Define prop interfaces explicitly for React components with proper typing of props and callback handlers
Files:
apps/studio/pages/project/[ref]/merge.tsxapps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsx
🧠 Learnings (5)
📚 Learning: 2025-12-12T05:20:17.409Z
Learnt from: joshenlim
Repo: supabase/supabase PR: 41258
File: apps/studio/pages/project/[ref]/storage/vectors/buckets/[bucketId].tsx:9-9
Timestamp: 2025-12-12T05:20:17.409Z
Learning: In apps/studio/**/*.{ts,tsx}, use named imports for DefaultLayout: import { DefaultLayout } from 'components/layouts/DefaultLayout' instead of default import. This is the new practice being adopted across the studio app.
Applied to files:
apps/studio/data/workflow-runs/keys.tsapps/studio/hooks/branches/useWorkflowManagement.tsapps/studio/pages/project/[ref]/merge.tsxapps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsxapps/studio/data/workflow-runs/workflow-run-logs-query.tsapps/studio/data/workflow-runs/workflow-run-query.ts
📚 Learning: 2026-01-02T05:58:55.226Z
Learnt from: meGauravJain
Repo: supabase/supabase PR: 41658
File: apps/studio/components/interfaces/Connect/content/prisma/content.tsx:70-85
Timestamp: 2026-01-02T05:58:55.226Z
Learning: In Prisma configuration or related connection setup code, prefer the environment variable name DIRECT_URL for direct/non-pooled database connections instead of DATABASE_URL_UNPOOLED, per Prisma documentation. Update code that reads database connection strings to use DIRECT_URL where applicable, and document why this is preferred (explicit, documented behavior). Ensure your environment provides DIRECT_URL when using direct connections and fallback handling if needed.
Applied to files:
apps/studio/data/workflow-runs/keys.tsapps/studio/hooks/branches/useWorkflowManagement.tsapps/studio/pages/project/[ref]/merge.tsxapps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsxapps/studio/data/workflow-runs/workflow-run-logs-query.tsapps/studio/data/workflow-runs/workflow-run-query.ts
📚 Learning: 2025-12-11T16:46:18.701Z
Learnt from: CR
Repo: supabase/supabase PR: 0
File: .cursor/rules/studio-best-practices.mdc:0-0
Timestamp: 2025-12-11T16:46:18.701Z
Learning: Applies to apps/studio/**/*.{ts,tsx} : Return objects from custom hooks instead of arrays for better extensibility and clearer API
Applied to files:
apps/studio/hooks/branches/useWorkflowManagement.ts
📚 Learning: 2025-12-11T16:46:18.701Z
Learnt from: CR
Repo: supabase/supabase PR: 0
File: .cursor/rules/studio-best-practices.mdc:0-0
Timestamp: 2025-12-11T16:46:18.701Z
Learning: Applies to apps/studio/**/*.tsx : Define prop interfaces explicitly for React components with proper typing of props and callback handlers
Applied to files:
apps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsx
📚 Learning: 2025-12-11T17:04:40.037Z
Learnt from: CR
Repo: supabase/supabase PR: 0
File: .cursor/rules/studio-ui.mdc:0-0
Timestamp: 2025-12-11T17:04:40.037Z
Learning: Applies to apps/studio/components/**/*.{ts,tsx} : Use CardContent for card sections, and CardFooter for actions within cards
Applied to files:
apps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsx
🧬 Code graph analysis (4)
apps/studio/hooks/branches/useWorkflowManagement.ts (2)
apps/studio/data/workflow-runs/workflow-run-query.ts (1)
useWorkflowRunQuery(45-58)apps/studio/data/workflow-runs/workflow-run-logs-query.ts (1)
useWorkflowRunLogsQuery(42-55)
apps/studio/pages/project/[ref]/merge.tsx (1)
apps/studio/hooks/branches/useWorkflowManagement.ts (1)
useWorkflowManagement(12-58)
apps/studio/data/workflow-runs/workflow-run-logs-query.ts (3)
apps/studio/data/graphql/graphql.ts (1)
Error(49-59)apps/studio/data/fetchers.ts (1)
handleError(146-184)apps/studio/data/workflow-runs/keys.ts (1)
workflowRunKeys(1-7)
apps/studio/data/workflow-runs/workflow-run-query.ts (2)
apps/studio/data/fetchers.ts (1)
handleError(146-184)apps/studio/data/workflow-runs/keys.ts (1)
workflowRunKeys(1-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: typecheck
- GitHub Check: test (1)
- GitHub Check: test
🔇 Additional comments (6)
apps/studio/data/workflow-runs/keys.ts (1)
5-6: LGTM!The new
statuskey follows the existing pattern and provides proper namespacing for status-specific queries, differentiating from thedetailkey used for logs.apps/studio/hooks/branches/useWorkflowManagement.ts (1)
39-47: LGTM!The effect correctly guards against calling
onWorkflowCompleteuntil the workflow has finished (status !== 'RUNNING'), ensuring the callback only receives 'SUCCESS' or 'FAILED'. ThehasRefetchedflag prevents duplicate calls.apps/studio/pages/project/[ref]/merge.tsx (1)
144-159: LGTM!Good refactoring to align with the updated hook return shape. The fallback logic (
currentBranchWorkflow || parentBranchWorkflow) correctly handles both workflow sources, and the renamed destructured variables improve clarity.apps/studio/data/workflow-runs/workflow-run-query.ts (1)
45-58: LGTM!The hook correctly uses the new
statuskey, passes bothprojectRefandworkflowRunId, and has proper enabled conditions checking both parameters are defined.apps/studio/data/workflow-runs/workflow-run-logs-query.ts (2)
12-37: LGTM!The function correctly validates required parameters, uses the new v1 endpoint, handles errors before returning, and returns a properly typed object. The
parseAs: 'text'option justifies the type assertion on line 36.
42-55: LGTM!The hook correctly uses
workflowRunKeys.detail(differentiating from thestatuskey used byuseWorkflowRunQuery) and has proper enabled conditions requiring bothprojectRefandworkflowRunId.
apps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsx
Outdated
Show resolved
Hide resolved
| const isSuccess = data?.run_steps.every((x) => ['EXITED', 'PAUSED'].includes(x.status)) | ||
| const isFailed = data?.run_steps.some((x) => x.status === 'DEAD') | ||
|
|
||
| // Return an object with the logs and we'll extract status from headers if needed | ||
| return { logs: data as string, workflowRunId } | ||
| if (error) handleError(error) | ||
| return { ...data, status: isSuccess ? 'SUCCESS' : isFailed ? 'FAILED' : 'RUNNING' } as WorkflowRun |
There was a problem hiding this comment.
Error handling occurs after accessing potentially undefined data.
Lines 35-36 access data?.run_steps before error handling on line 38. If the API returns an error, data may be undefined, and while optional chaining prevents a crash, isSuccess and isFailed will be undefined, leading to incorrect status derivation. Move error handling before data processing.
Reorder error handling
const { data, error } = await get('/v1/projects/{ref}/actions/{run_id}', {
params: {
path: {
ref: projectRef,
run_id: workflowRunId,
},
},
signal,
})
+ if (error) handleError(error)
+
const isSuccess = data?.run_steps.every((x) => ['EXITED', 'PAUSED'].includes(x.status))
const isFailed = data?.run_steps.some((x) => x.status === 'DEAD')
- if (error) handleError(error)
return { ...data, status: isSuccess ? 'SUCCESS' : isFailed ? 'FAILED' : 'RUNNING' } as WorkflowRunCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/studio/data/workflow-runs/workflow-run-query.ts around lines 35 - 39,
The status computation uses data?.run_steps (isSuccess/isFailed) before checking
for an API error, which can yield undefined results; move the error handling so
that the error is checked and handled (handleError(error)) before computing
isSuccess and isFailed, then compute status from data.run_steps (or return early
if data is undefined) and finally return the WorkflowRun object; update
references in this block including data, error, isSuccess, isFailed, run_steps
and the final return to reflect the reordered/guarded logic.
🎭 Playwright Test Results
Details
Flaky testsFeatures › sql-editor.spec.ts › SQL Editor › should check if SQL editor is working as expected Skipped testsFeatures › sql-editor.spec.ts › SQL Editor › snippet favourite works as expected |
…-in-creating_project-status
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/studio/data/actions/action-detail-query.ts`:
- Around line 45-52: The query is being enabled for empty strings because
enabled only checks for undefined; fix by extracting descriptive booleans (e.g.,
hasValidProjectRef = typeof projectRef !== 'undefined' && projectRef !== '' and
hasValidRunId = typeof runId !== 'undefined' && runId !== '') and then use a
combined shouldEnable (enabled && hasValidProjectRef && hasValidRunId) in the
useQuery call (reference actionKeys.detail, getActionRun and the query's enabled
option) so empty strings won't trigger getActionRun; keep the new variables
local to the function for clarity.
In `@apps/studio/data/actions/action-logs-query.ts`:
- Around line 38-47: The query enablement currently only checks for undefined
but getActionRunLogs throws on empty strings; update the enabled condition used
in the useQuery call (queryKey: actionKeys.logs, queryFn: getActionRunLogs) to
validate runtime inputs by verifying projectRef and runId are non-empty (e.g.,
typeof !== 'undefined' && projectRef !== '' && runId !== ''), and extract that
multi-part validation into a descriptive boolean variable (e.g.,
hasValidIdentifiers) declared above the useQuery so the enabled flag uses that
variable instead of the current check; ensure you reference projectRef, runId,
getActionRunLogs, and actionKeys.logs when making the change.
🧹 Nitpick comments (4)
apps/studio/components/ui/DiffEditor.tsx (1)
4-16: Consider renaming interface to match exported component name.The interface is named
DiffViewerPropsbut the component is now exported asDiffEditor. For consistency and clarity, consider renaming toDiffEditorProps.Suggested rename
-interface DiffViewerProps { +interface DiffEditorProps { /** Original/left hand side content (optional) */ original?: string /** Modified/right hand side content */ modified: string | undefined /** Language identifier understood by Monaco */ language?: string /** Height for the editor container */ height?: string | number /** Diff Editor Options */ options?: monacoEditor.IStandaloneDiffEditorConstructionOptions onMount?: (editor: monacoEditor.IStandaloneDiffEditor) => void }And update the component signature:
export const DiffEditor = ({ ... -}: DiffViewerProps) => ( +}: DiffEditorProps) => (apps/studio/components/interfaces/BranchManagement/DatabaseDiffPanel.tsx (1)
8-14: Unused propshowRefreshButtonin interface.The
showRefreshButtonprop is defined inDatabaseDiffPanelPropsbut is not destructured or used in the component implementation. Consider removing it if it's no longer needed, or implement its functionality if it was intended.If unused, remove from interface
interface DatabaseDiffPanelProps { diffContent?: string isLoading: boolean error?: any - showRefreshButton?: boolean currentBranchRef?: string }apps/studio/components/interfaces/BranchManagement/EdgeFunctionsDiffPanel.tsx (1)
207-223: Commented code references removed propmainBranchRef.The commented-out code for removed functions (line 217) still references
mainBranchRef, which has been removed from the component's props interface. If this feature is to be implemented later, the prop would need to be re-added. Consider either updating the comment to reflect the current API or removing the dead code entirely.Option 1: Remove the commented code entirely
))} </div> )} - {/* TODO: Removing functions is not supported yet */} - {/* {diffResults.removedSlugs.length > 0 && ( - <div> - <div className="space-y-4"> - {diffResults.removedSlugs.map((slug) => ( - <FunctionDiff - key={slug} - functionSlug={slug} - currentBody={EMPTY_ARR} - mainBody={diffResults.removedBodiesMap[slug]!} - currentBranchRef={mainBranchRef} - fileInfos={diffResults.functionFileInfo[slug] || EMPTY_ARR} - /> - ))} - </div> - </div> - )} */} {diffResults.modifiedSlugs.length > 0 && (Option 2: Add a note about the missing prop
- {/* TODO: Removing functions is not supported yet */} + {/* TODO: Removing functions is not supported yet. Note: mainBranchRef prop was removed and would need to be re-added. */}apps/studio/data/actions/action-detail-query.ts (1)
35-38: Avoid theas ActionRunassertion.
Prefer returning a typed value directly to avoid type casting.As per coding guidelines, avoid type casting in favor of runtime-safe typing.♻️ Suggested update
-export async function getActionRun( - { projectRef, runId }: ActionRunVariables, - signal?: AbortSignal -) { +export async function getActionRun( + { projectRef, runId }: ActionRunVariables, + signal?: AbortSignal +): Promise<ActionRun> { @@ - return { - ...data, - status: isRunning ? 'RUNNING' : isSuccess ? 'SUCCESS' : isFailed ? 'FAILED' : 'RUNNING', - } as ActionRun + const status = isRunning ? 'RUNNING' : isSuccess ? 'SUCCESS' : isFailed ? 'FAILED' : 'RUNNING' + return { ...data, status } }
| { projectRef, runId }: ActionRunVariables, | ||
| { enabled = true, ...options }: UseCustomQueryOptions<ActionRunData, ActionRunError, TData> = {} | ||
| ) => | ||
| useQuery<ActionRunData, ActionRunError, TData>({ | ||
| queryKey: actionKeys.detail(ref, run_id), | ||
| queryFn: ({ signal }) => getActionRun({ ref, run_id }, signal), | ||
| enabled: enabled && Boolean(ref) && Boolean(run_id), | ||
| queryKey: actionKeys.detail(projectRef, runId), | ||
| queryFn: ({ signal }) => getActionRun({ projectRef, runId }, signal), | ||
| enabled: enabled && typeof projectRef !== 'undefined' && typeof runId !== 'undefined', | ||
| staleTime: 0, |
There was a problem hiding this comment.
Align query enablement with runtime validation.
getActionRun throws on empty strings, but enabled only checks for undefined, so empty strings still trigger the query and error.
✅ Suggested fix
-export const useActionRunQuery = <TData = ActionRunData>(
- { projectRef, runId }: ActionRunVariables,
- { enabled = true, ...options }: UseCustomQueryOptions<ActionRunData, ActionRunError, TData> = {}
-) =>
- useQuery<ActionRunData, ActionRunError, TData>({
+export const useActionRunQuery = <TData = ActionRunData>(
+ { projectRef, runId }: ActionRunVariables,
+ { enabled = true, ...options }: UseCustomQueryOptions<ActionRunData, ActionRunError, TData> = {}
+) => {
+ const hasActionRunParams = Boolean(projectRef && runId)
+ return useQuery<ActionRunData, ActionRunError, TData>({
queryKey: actionKeys.detail(projectRef, runId),
queryFn: ({ signal }) => getActionRun({ projectRef, runId }, signal),
- enabled: enabled && typeof projectRef !== 'undefined' && typeof runId !== 'undefined',
+ enabled: enabled && hasActionRunParams,
staleTime: 0,
...options,
})
+}🤖 Prompt for AI Agents
In `@apps/studio/data/actions/action-detail-query.ts` around lines 45 - 52, The
query is being enabled for empty strings because enabled only checks for
undefined; fix by extracting descriptive booleans (e.g., hasValidProjectRef =
typeof projectRef !== 'undefined' && projectRef !== '' and hasValidRunId =
typeof runId !== 'undefined' && runId !== '') and then use a combined
shouldEnable (enabled && hasValidProjectRef && hasValidRunId) in the useQuery
call (reference actionKeys.detail, getActionRun and the query's enabled option)
so empty strings won't trigger getActionRun; keep the new variables local to the
function for clarity.
| { projectRef, runId }: ActionLogsVariables, | ||
| { | ||
| enabled = true, | ||
| ...options | ||
| }: UseCustomQueryOptions<ActionLogsData, WorkflowRunLogsError, TData> = {} | ||
| ) => | ||
| useQuery<ActionLogsData, WorkflowRunLogsError, TData>({ | ||
| queryKey: actionKeys.detail(ref, run_id), | ||
| queryFn: ({ signal }) => getActionRunLogs({ ref, run_id }, signal), | ||
| enabled: enabled && Boolean(ref) && Boolean(run_id), | ||
| queryKey: actionKeys.logs(projectRef, runId), | ||
| queryFn: ({ signal }) => getActionRunLogs({ projectRef, runId }, signal), | ||
| enabled: enabled && typeof projectRef !== 'undefined' && typeof runId !== 'undefined', |
There was a problem hiding this comment.
Align query enablement with runtime validation.
getActionRunLogs throws on empty strings, but enabled only guards undefined.
✅ Suggested fix
-export const useActionRunLogsQuery = <TData = ActionLogsData>(
- { projectRef, runId }: ActionLogsVariables,
- {
- enabled = true,
- ...options
- }: UseCustomQueryOptions<ActionLogsData, WorkflowRunLogsError, TData> = {}
-) =>
- useQuery<ActionLogsData, WorkflowRunLogsError, TData>({
+export const useActionRunLogsQuery = <TData = ActionLogsData>(
+ { projectRef, runId }: ActionLogsVariables,
+ {
+ enabled = true,
+ ...options
+ }: UseCustomQueryOptions<ActionLogsData, WorkflowRunLogsError, TData> = {}
+) => {
+ const hasActionRunParams = Boolean(projectRef && runId)
+ return useQuery<ActionLogsData, WorkflowRunLogsError, TData>({
queryKey: actionKeys.logs(projectRef, runId),
queryFn: ({ signal }) => getActionRunLogs({ projectRef, runId }, signal),
- enabled: enabled && typeof projectRef !== 'undefined' && typeof runId !== 'undefined',
+ enabled: enabled && hasActionRunParams,
staleTime: 0,
...options,
})
+}🤖 Prompt for AI Agents
In `@apps/studio/data/actions/action-logs-query.ts` around lines 38 - 47, The
query enablement currently only checks for undefined but getActionRunLogs throws
on empty strings; update the enabled condition used in the useQuery call
(queryKey: actionKeys.logs, queryFn: getActionRunLogs) to validate runtime
inputs by verifying projectRef and runId are non-empty (e.g., typeof !==
'undefined' && projectRef !== '' && runId !== ''), and extract that multi-part
validation into a descriptive boolean variable (e.g., hasValidIdentifiers)
declared above the useQuery so the enabled flag uses that variable instead of
the current check; ensure you reference projectRef, runId, getActionRunLogs, and
actionKeys.logs when making the change.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/studio/components/interfaces/BranchManagement/EdgeFunctionsDiffPanel.tsx (1)
3-34: Use a lightweight basename implementation to avoid importing Nodepathin client-side components.The codebase already has
path-browserifyas a transitive dependency and Next.js handles Node module polyfills automatically, but the related hookuseEdgeFunctionsDiff.tsexplicitly acknowledges this concern with a comment: "avoids importing the full Node path lib for the browser bundle". For consistency and to follow the pattern already established in the codebase, replace the Nodepathimport with a simple inline helper:-import { basename } from 'path' - -const fileKey = (fullPath: string) => basename(fullPath) +const fileKey = (fullPath: string) => { + const normalized = fullPath.replace(/\\/g, '/') + return normalized.slice(normalized.lastIndexOf('/') + 1) +}This aligns with the approach already used in
useEdgeFunctionsDiff.tsand removes an unnecessary dependency from the client bundle.apps/studio/components/interfaces/BranchManagement/WorkflowLogs.tsx (1)
7-7: Fix build-breaking selectedWorkflowRun type mismatch.TS2345 at Line 112 is caused by
setSelectedWorkflowRun(workflowRun)feeding a list item withoutstatusinto a state typed asActionRunData. Align the state type with the list item (or derivestatus) to unblock the build.🐛 Proposed fix (align state with list item type)
-import { ActionRunData } from 'data/actions/action-detail-query' +// ActionRunData no longer needed once state aligns with list items -const [selectedWorkflowRun, setSelectedWorkflowRun] = useState<ActionRunData | undefined>( - undefined -) +const [selectedWorkflowRun, setSelectedWorkflowRun] = useState< + NonNullable<typeof workflowRuns>[number] | undefined +>(undefined)Also applies to: 53-55, 109-113
🧹 Nitpick comments (2)
apps/studio/data/actions/action-detail-query.ts (1)
17-38: Avoidas ActionRuncast; return a typed object instead.Prefer a typed return + explicit
statusvariable to avoid casting. As per coding guidelines, avoidas Typewhere possible.♻️ Proposed refactor
export async function getActionRun( { projectRef, runId }: ActionRunVariables, signal?: AbortSignal -) { +): Promise<ActionRun> { @@ - return { - ...data, - status: isRunning ? 'RUNNING' : isSuccess ? 'SUCCESS' : isFailed ? 'FAILED' : 'RUNNING', - } as ActionRun + const status: ActionRun['status'] = + isRunning ? 'RUNNING' : isSuccess ? 'SUCCESS' : isFailed ? 'FAILED' : 'RUNNING' + + return { ...data, status } }apps/studio/components/interfaces/BranchManagement/WorkflowLogsCard.tsx (1)
6-6: Move nested ternary out of JSX for readability.Assign the class to a descriptive variable to avoid nested ternaries in JSX, as per coding guidelines.
♻️ Proposed refactor
-import { Button, Card, CardContent, CardHeader, CardTitle } from 'ui' +import { Button, Card, CardContent, CardHeader, CardTitle, cn } from 'ui' @@ - return ( + const headerClassName = cn({ + 'text-brand': isSuccess, + 'text-destructive': isFailed, + }) + + return ( <Card className="bg-background overflow-hidden h-64 flex flex-col"> - <CardHeader className={isSuccess ? 'text-brand' : isFailed ? 'text-destructive' : ''}> + <CardHeader className={headerClassName}>Also applies to: 69-69
awaseem
left a comment
awaseem
left a comment
There was a problem hiding this comment.
Put some comments in the slack thread, Just need to test to make sure this is working correctly
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/studio/pages/project/[ref]/merge.tsx (1)
114-131: Gate review request reset on successful workflow completion.
updateBranch({ requestReview: false })currently fires for bothSUCCESSandFAILEDstatuses. This should only execute onstatus === 'SUCCESS'— clearing the review request when a workflow fails removes the review flag that developers need to address failures and re-request review.✅ Suggested fix
- if (ref && parentProjectRef && currentBranch?.review_requested_at) { + if (status === 'SUCCESS' && ref && parentProjectRef && currentBranch?.review_requested_at) { updateBranch( { branchRef: ref, projectRef: parentProjectRef, requestReview: false, },
joshenlim
deleted the
joshen/fe-2348-gitless-branch-merges-stuck-in-creating_project-status
branch
Braintrust eval report
|
3 participants
Context
This is only for Gitless branching - merge attempts get stuck at "Creating project" in the logs. This is due to the wrong endpoint being used as we now need to use
v1endpoints as opposed to theplatformendpoints.Changes in this PR fixes that such that the workflow logs populate correctly.
Other changes involved
workflow-runsreact query -> Branching now uses theactionsreact queryactionsto be more consistent with how we write our RQsDiffEditorcomponents to import fromuiinstead ofmonacodirectlyDiffEditorinuiconsolidates the options, rather than declaring the options in separate places whenever we renderDiffEditorTextModel got disposed before DiffEditorWidget model got resetwhen switching between "Database" and "Edge Functions" in the branch merge pageTo test
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.