Open
Conversation
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:
📝 WalkthroughWalkthroughThis PR adds per-app build timeout support: schema and migration add ChangesBuild Timeout Feature
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Capgo API
participant DB as Supabase
participant Builder as Builder Service
Client->>API: POST /build (create build)
API->>DB: SELECT apps.build_timeout_seconds
DB-->>API: app timeout
API->>API: normalize timeout, call buildBuilderPayload(timeoutSeconds)
API->>Builder: POST build job (buildOptions.timeoutSeconds)
Builder-->>API: job_id
API->>DB: INSERT build_requests including build_timeout_seconds
API-->>Client: Response { timeout_seconds }
Client->>API: GET /build/status
API->>DB: SELECT build_requests including build_timeout_seconds
DB-->>API: request row + timeout
API->>API: compute runtime vs timeout
alt timed out & not terminal
API->>Builder: POST /cancel (best-effort)
Builder-->>API: cancel response
API->>DB: UPDATE build_requests with timeout-adjusted status/error
else
API->>DB: UPDATE build_requests with builder status/error
end
API-->>Client: status response (effective values, capped runtime)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
riderx
force-pushed
the
codex/build-timeout
branch
from
711dcff to
44e0bb6
Compare
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 3
🧹 Nitpick comments (2)
tests/builder-payload.unit.test.ts (1)
83-94: ⚡ Quick winUse
it.concurrent()per the test coding guideline.The new test is a stateless unit test with no shared state, so it can safely run concurrently. The new companion file
tests/build-timeout.unit.test.tsalready follows this convention.♻️ Proposed fix
-it('injects the app timeout into buildOptions', () => { + it.concurrent('injects the app timeout into buildOptions', () => {As per coding guidelines: "Use
it.concurrent()instead ofit()when possible to run tests in parallel within the same file."🤖 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 `@tests/builder-payload.unit.test.ts` around lines 83 - 94, The test case that verifies injecting app timeout into buildOptions should be converted to run concurrently: change the test declaration using it('injects the app timeout into buildOptions', ...) to it.concurrent('injects the app timeout into buildOptions', ...) in tests/builder-payload.unit.test.ts so the stateless unit test (which calls buildBuilderPayload) can run in parallel with other tests following the project guideline.src/components/dashboard/AppSetting.vue (1)
463-483: ⚡ Quick winPrefer
throw/returnoverreturn Promise.reject()/return Promise.resolve()in async functions.SonarCloud flags all four occurrences (lines 466, 470, 474, 478). Because
updateBuildTimeoutisasync, these are semantically identical butthrow/returnis the idiomatic form. Note: since thecatchblock casts the error asstring(toast.error(error as string)), throw the translated string directly rather than wrapping innew Error(...).♻️ Proposed fix
async function updateBuildTimeout(rawTimeoutMinutes: number | string | undefined) { const timeoutMinutes = Number(rawTimeoutMinutes) if (!Number.isFinite(timeoutMinutes)) - return Promise.reject(t('build-timeout-invalid')) + throw t('build-timeout-invalid') const normalizedMinutes = Math.trunc(timeoutMinutes) if (normalizedMinutes < MIN_BUILD_TIMEOUT_MINUTES || normalizedMinutes > MAX_BUILD_TIMEOUT_MINUTES) - return Promise.reject(t('build-timeout-invalid')) + throw t('build-timeout-invalid') const timeoutSeconds = normalizedMinutes * 60 if (timeoutSeconds === appRef.value?.build_timeout_seconds) - return Promise.resolve() + return const { error } = await supabase.from('apps').update({ build_timeout_seconds: timeoutSeconds }).eq('app_id', props.appId) if (error) - return Promise.reject(t('cannot-change-build-timeout')) + throw t('cannot-change-build-timeout')🤖 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 `@src/components/dashboard/AppSetting.vue` around lines 463 - 483, The async function updateBuildTimeout uses Promise.reject/Promise.resolve; change these to idiomatic async patterns: throw the translated error strings (e.g., throw t('build-timeout-invalid')) instead of return Promise.reject(...), use bare return instead of return Promise.resolve() for the no-op path, and throw t('cannot-change-build-timeout') on supabase errors; keep the toast.success call and the appRef.value.build_timeout_seconds assignment unchanged so behavior is preserved.
🤖 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 `@supabase/functions/_backend/public/build/status.ts`:
- Around line 36-74: The cancelTimedOutBuilderJob call can block getBuildStatus
because fetch has no timeout; add an AbortController-based timeout inside
cancelTimedOutBuilderJob (e.g., 1–3s): create an AbortController, start a timer
to call controller.abort() after the timeout, pass controller.signal to fetch,
clear the timer after fetch completes, and treat abort errors as non-fatal (log
and return). Alternatively, if cancellation should never block getBuildStatus,
invoke cancelTimedOutBuilderJob without awaiting it from getBuildStatus so the
status response is returned immediately; use one of these two approaches and
keep error logging in cloudlogErr as currently implemented.
In `@supabase/functions/_backend/triggers/cron_reconcile_build_status.ts`:
- Around line 36-41: The cancelTimedOutBuilderJob function currently uses
fetch() with no timeout and can hang; update it to use an AbortController and a
short timeout (e.g., 3-5s): create an AbortController, pass controller.signal to
fetch(`${builderUrl}/jobs/${jobId}/cancel`), set a timer to call
controller.abort() after the timeout, clear the timer on fetch completion, and
ensure the function still returns the fetch Response (or propagates the abort
error) so callers in Promise.allSettled won't block indefinitely.
In `@supabase/migrations/20260507130744_build_timeout_seconds.sql`:
- Around line 49-59: The call to rbac_check_permission_direct uses positional
args in the wrong order; reorder them to match the function signature (p_apikey,
p_app_id, p_channel_id, p_org_id, p_permission_key, p_user_id) — pass
v_request_apikey first, then a.app_id, then NULL for channel, then a.owner_org,
then public.rbac_perm_app_read(), and finally v_api_key.user_id so
rbac_check_permission_direct(...) is invoked with the correct argument mapping.
---
Nitpick comments:
In `@src/components/dashboard/AppSetting.vue`:
- Around line 463-483: The async function updateBuildTimeout uses
Promise.reject/Promise.resolve; change these to idiomatic async patterns: throw
the translated error strings (e.g., throw t('build-timeout-invalid')) instead of
return Promise.reject(...), use bare return instead of return Promise.resolve()
for the no-op path, and throw t('cannot-change-build-timeout') on supabase
errors; keep the toast.success call and the appRef.value.build_timeout_seconds
assignment unchanged so behavior is preserved.
In `@tests/builder-payload.unit.test.ts`:
- Around line 83-94: The test case that verifies injecting app timeout into
buildOptions should be converted to run concurrently: change the test
declaration using it('injects the app timeout into buildOptions', ...) to
it.concurrent('injects the app timeout into buildOptions', ...) in
tests/builder-payload.unit.test.ts so the stateless unit test (which calls
buildBuilderPayload) can run in parallel with other tests following the project
guideline.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 212c8a3e-e8a9-44bb-b793-8ad9763f4bdb
📒 Files selected for processing (13)
cli/src/types/supabase.types.tsmessages/en.jsonsrc/components/dashboard/AppSetting.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/build/request.tssupabase/functions/_backend/public/build/status.tssupabase/functions/_backend/triggers/cron_reconcile_build_status.tssupabase/functions/_backend/utils/build_timeout.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260507130744_build_timeout_seconds.sqltests/build-timeout.unit.test.tstests/builder-payload.unit.test.ts
riderx
force-pushed
the
codex/build-timeout
branch
from
44e0bb6 to
07bb1e3
Compare
![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: 07bb1e3cc3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
supabase/functions/_backend/public/build/status.ts (1)
222-235: 💤 Low valueConsider extracting the nested ternary for
build_time_seconds.SonarCloud flags the nested ternary at lines 228-232. While the current form is understandable, extracting to a local variable would improve readability and silence the warning.
Proposed refactor
+ const responseBuildTimeSeconds = timedOut + ? cappedBuildTimeSeconds + : builderJob.job.started_at && builderJob.job.completed_at + ? runtimeSeconds + : null + return c.json({ job_id, status: effectiveStatus, machine: builderJob.machine || null, started_at: builderJob.job.started_at, completed_at: effectiveCompletedAt, - build_time_seconds: timedOut - ? cappedBuildTimeSeconds - : builderJob.job.started_at && builderJob.job.completed_at - ? runtimeSeconds - : null, + build_time_seconds: responseBuildTimeSeconds, error: effectiveError, upload_url: builderJob.uploadUrl || null, }, 200)🤖 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 `@supabase/functions/_backend/public/build/status.ts` around lines 222 - 235, Extract the nested ternary used for the build_time_seconds field into a local variable (e.g., buildTimeSeconds) before the return so the return object simply uses build_time_seconds: buildTimeSeconds; compute buildTimeSeconds by first checking timedOut to return cappedBuildTimeSeconds, then if builderJob.job.started_at && builderJob.job.completed_at return runtimeSeconds, else return null—this removes the nested ternary and makes the logic in the return (in status.ts) clearer and SonarCloud-friendly.
🤖 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 `@tests/build-timeout.unit.test.ts`:
- Around line 2-12: The named imports are out of alphabetical order for ESLint:
move calculateBuildRuntimeSeconds so it appears before MIN_BUILD_TIMEOUT_SECONDS
in the import list; ensure the import specifier order in the import statement
that includes DEFAULT_BUILD_TIMEOUT_SECONDS, MAX_BUILD_TIMEOUT_SECONDS,
MIN_BUILD_TIMEOUT_SECONDS, calculateBuildRuntimeSeconds,
calculateTimeoutCompletedAt, capBuildRuntimeSeconds, formatBuildTimeoutError,
hasBuildTimedOut, normalizeBuildTimeoutSeconds is alphabetically sorted (so
calculateBuildRuntimeSeconds comes before MIN_BUILD_TIMEOUT_SECONDS) to satisfy
the linter.
In `@tests/builder-payload.unit.test.ts`:
- Around line 82-83: Remove the extra blank line inside the test case named
"injects the app timeout into buildOptions" in
tests/builder-payload.unit.test.ts so the test body has no consecutive blank
lines (ensure only a single blank line separates logical blocks), fixing the
ESLint "More than 1 blank line not allowed" error; locate the
it.concurrent('injects the app timeout into buildOptions', ...) block and delete
the surplus empty line so formatting conforms to lint rules.
---
Nitpick comments:
In `@supabase/functions/_backend/public/build/status.ts`:
- Around line 222-235: Extract the nested ternary used for the
build_time_seconds field into a local variable (e.g., buildTimeSeconds) before
the return so the return object simply uses build_time_seconds:
buildTimeSeconds; compute buildTimeSeconds by first checking timedOut to return
cappedBuildTimeSeconds, then if builderJob.job.started_at &&
builderJob.job.completed_at return runtimeSeconds, else return null—this removes
the nested ternary and makes the logic in the return (in status.ts) clearer and
SonarCloud-friendly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe796aeb-1154-45c8-bcae-c12b56a1dc3a
📒 Files selected for processing (13)
cli/src/types/supabase.types.tsmessages/en.jsonsrc/components/dashboard/AppSetting.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/build/request.tssupabase/functions/_backend/public/build/status.tssupabase/functions/_backend/triggers/cron_reconcile_build_status.tssupabase/functions/_backend/utils/build_timeout.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260507130744_build_timeout_seconds.sqltests/build-timeout.unit.test.tstests/builder-payload.unit.test.ts
✅ Files skipped from review due to trivial changes (5)
- supabase/functions/_backend/utils/postgres_schema.ts
- cli/src/types/supabase.types.ts
- supabase/functions/_backend/utils/supabase.types.ts
- src/types/supabase.types.ts
- src/components/dashboard/AppSetting.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- messages/en.json
- supabase/functions/_backend/public/build/request.ts
riderx
force-pushed
the
codex/build-timeout
branch
from
07bb1e3 to
bc9a209
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/dashboard/AppSetting.vue (1)
91-92: 💤 Low valueMinor display/save mismatch:
Math.roundvsMath.trunc.The computed
buildTimeoutMinutesusesMath.round()to display the value, butupdateBuildTimeoutusesMath.trunc()to normalize before saving. If a user somehow has a non-integer value stored (e.g., 901 seconds = 15.0167 minutes), the display would show 15 but saving would preserve 15.This is unlikely to cause issues since the DB column and all writes use integer seconds divisible by 60, but aligning both to
Math.trunc(orMath.round) would be more consistent.🤖 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 `@src/components/dashboard/AppSetting.vue` around lines 91 - 92, The computed buildTimeoutMinutes currently uses Math.round while updateBuildTimeout normalizes with Math.trunc; make them consistent by changing buildTimeoutMinutes to use Math.trunc((appRef.value?.build_timeout_seconds ?? DEFAULT_BUILD_TIMEOUT_SECONDS) / 60) so display and save logic (buildTimeoutMinutes and updateBuildTimeout) behave identically when fractional minutes exist.
🤖 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.
Nitpick comments:
In `@src/components/dashboard/AppSetting.vue`:
- Around line 91-92: The computed buildTimeoutMinutes currently uses Math.round
while updateBuildTimeout normalizes with Math.trunc; make them consistent by
changing buildTimeoutMinutes to use
Math.trunc((appRef.value?.build_timeout_seconds ??
DEFAULT_BUILD_TIMEOUT_SECONDS) / 60) so display and save logic
(buildTimeoutMinutes and updateBuildTimeout) behave identically when fractional
minutes exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c0bb7ff0-0775-4707-970c-0ea08f9b7f1a
📒 Files selected for processing (13)
cli/src/types/supabase.types.tsmessages/en.jsonsrc/components/dashboard/AppSetting.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/build/request.tssupabase/functions/_backend/public/build/status.tssupabase/functions/_backend/triggers/cron_reconcile_build_status.tssupabase/functions/_backend/utils/build_timeout.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260507130744_build_timeout_seconds.sqltests/build-timeout.unit.test.tstests/builder-payload.unit.test.ts
✅ Files skipped from review due to trivial changes (4)
- supabase/functions/_backend/utils/postgres_schema.ts
- cli/src/types/supabase.types.ts
- supabase/functions/_backend/utils/supabase.types.ts
- supabase/functions/_backend/utils/build_timeout.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/builder-payload.unit.test.ts
- src/types/supabase.types.ts
- supabase/functions/_backend/public/build/status.ts
riderx
force-pushed
the
codex/build-timeout
branch
from
bc9a209 to
d9ea419
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9ea419f58
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@supabase/functions/_backend/public/build/status.ts`:
- Around line 154-161: The code currently flips timeoutApplied based on the
success of cancelTimedOutBuilderJob, which hides the true timedOut state; change
the logic so timeoutApplied remains equal to timedOut and call
cancelTimedOutBuilderJob only as a best-effort side-effect (do not mutate
timeoutApplied based on its return). Concretely: keep timeoutApplied = timedOut,
invoke cancelTimedOutBuilderJob(c, job_id, buildRequest.app_id) but do not
assign its result to timeoutApplied (either await it but ignore the result or
fire-and-forget the Promise), and continue using timeoutApplied to compute
effectiveStatus (BUILD_TIMEOUT_STATUS), effectiveError
(formatBuildTimeoutError), and effectiveCompletedAt (timeoutCompletedAt)
regardless of cancelTimedOutBuilderJob outcome.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca1ab326-2f0f-4423-a741-586b84424b35
📒 Files selected for processing (13)
cli/src/types/supabase.types.tsmessages/en.jsonsrc/components/dashboard/AppSetting.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/build/request.tssupabase/functions/_backend/public/build/status.tssupabase/functions/_backend/triggers/cron_reconcile_build_status.tssupabase/functions/_backend/utils/build_timeout.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260507130744_build_timeout_seconds.sqltests/build-timeout.unit.test.tstests/builder-payload.unit.test.ts
✅ Files skipped from review due to trivial changes (7)
- messages/en.json
- supabase/functions/_backend/utils/postgres_schema.ts
- src/types/supabase.types.ts
- tests/build-timeout.unit.test.ts
- supabase/functions/_backend/utils/supabase.types.ts
- cli/src/types/supabase.types.ts
- supabase/functions/_backend/utils/build_timeout.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/builder-payload.unit.test.ts
- supabase/functions/_backend/triggers/cron_reconcile_build_status.ts
riderx
force-pushed
the
codex/build-timeout
branch
from
d9ea419 to
220ae66
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supabase/functions/_backend/triggers/cron_reconcile_build_status.ts (1)
154-154: 💤 Low valueTimeout counter only increments on successful cancel.
timedOut++is inside the cancel block, so it only counts builds where cancellation was attempted and succeeded. Consider moving the increment outside the cancel condition to count all timed-out builds, or rename the metric to clarify it represents "cancelled timed-out builds".Option A: Count all timed-out builds
if (buildTimedOut && !isTerminalBuildStatus(jobStatus)) { const cancelResponse = await cancelTimedOutBuilderJob(...) // ... error handling ... - timedOut++ } + if (buildTimedOut) + timedOut++🤖 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 `@supabase/functions/_backend/triggers/cron_reconcile_build_status.ts` at line 154, The timedOut counter is incremented only when cancellation succeeds because timedOut++ is inside the cancel-success block; move the timedOut++ increment out of that conditional so it runs whenever a build is detected as timed-out (i.e., when you mark a build as timed out in the reconcile loop) rather than only on successful cancelation, or alternatively rename timedOut to something like cancelledTimedOut to reflect current behavior; update references to the timedOut variable accordingly (look for the timedOut variable and the cancel logic/cancel block in cron_reconcile_build_status.ts).
🤖 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 `@supabase/functions/_backend/triggers/cron_reconcile_build_status.ts`:
- Around line 140-155: The cancel failure currently throws inside the
buildTimedOut block which aborts the rest of the reconciliation and prevents
updating build_requests to BUILD_TIMEOUT_STATUS; change this to a best-effort
cancel: call cancelTimedOutBuilderJob(builderUrl, builderApiKey,
build.builder_job_id!) and if cancelResponse.ok is false, log the error with
cloudlogErr (including requestId, buildId, jobId, status, error text) but do NOT
throw—allow execution to continue so the code that updates the build row to
BUILD_TIMEOUT_STATUS still runs; ensure timedOut counting and any subsequent
update logic that sets BUILD_TIMEOUT_STATUS for the build (the code that uses
isTerminalBuildStatus and the build_requests update) still executes even when
cancel fails.
---
Nitpick comments:
In `@supabase/functions/_backend/triggers/cron_reconcile_build_status.ts`:
- Line 154: The timedOut counter is incremented only when cancellation succeeds
because timedOut++ is inside the cancel-success block; move the timedOut++
increment out of that conditional so it runs whenever a build is detected as
timed-out (i.e., when you mark a build as timed out in the reconcile loop)
rather than only on successful cancelation, or alternatively rename timedOut to
something like cancelledTimedOut to reflect current behavior; update references
to the timedOut variable accordingly (look for the timedOut variable and the
cancel logic/cancel block in cron_reconcile_build_status.ts).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9db00d94-7e5d-44be-a3a7-b2a8d24bc0e1
📒 Files selected for processing (13)
cli/src/types/supabase.types.tsmessages/en.jsonsrc/components/dashboard/AppSetting.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/build/request.tssupabase/functions/_backend/public/build/status.tssupabase/functions/_backend/triggers/cron_reconcile_build_status.tssupabase/functions/_backend/utils/build_timeout.tssupabase/functions/_backend/utils/postgres_schema.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260507130744_build_timeout_seconds.sqltests/build-timeout.unit.test.tstests/builder-payload.unit.test.ts
✅ Files skipped from review due to trivial changes (7)
- supabase/functions/_backend/utils/postgres_schema.ts
- tests/build-timeout.unit.test.ts
- supabase/functions/_backend/utils/supabase.types.ts
- messages/en.json
- src/components/dashboard/AppSetting.vue
- supabase/migrations/20260507130744_build_timeout_seconds.sql
- supabase/functions/_backend/utils/build_timeout.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/builder-payload.unit.test.ts
- cli/src/types/supabase.types.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 220ae66935
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
riderx
force-pushed
the
codex/build-timeout
branch
from
220ae66 to
52c8f77
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
riderx
force-pushed
the
codex/build-timeout
branch
2 times, most recently
from
65ac706 to
a063ab7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a063ab7dbe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
riderx
force-pushed
the
codex/build-timeout
branch
from
a063ab7 to
b96e495
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b96e495628
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
riderx
force-pushed
the
codex/build-timeout
branch
from
b96e495 to
d637e99
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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 (AI generated)
timeoutSecondsfrom the builder payload so clients cannot override the app setting.Motivation (AI generated)
A stuck or bad native builder job could report an excessive runtime and consume user build credits. Native cloud builds need a bounded runtime that users can tune per app, and changing the app setting should affect builds that are already running.
Business Impact (AI generated)
This reduces billing disputes from runaway native builds, protects customer credits, and gives support a clear timeout control for unusual projects.
Test Plan (AI generated)
bun lintbun lint:backendbunx vitest run tests/build-timeout.unit.test.ts tests/builder-payload.unit.test.tsbun typecheckgit diff --checkGenerated with AI