fix(api): un-quote orderBy column refs in sandbox lifecycle crons by lilongen · Pull Request #672 · boxlite-ai/boxlite

lilongen · 2026-06-08T04:59:01Z

Summary

The sandbox auto-stop / auto-archive / auto-delete cron queries passed pre-quoted column refs (sandbox."lastBackupAt", activity."lastActivityAt") to QueryBuilder.orderBy(...). TypeORM interprets the alias.property form and applies its own identifier quoting, so the already-quoted column gets double-escaped into invalid SQL and the cron crashes.

Fix: pass the plain alias.property and let TypeORM map + quote it. CustomNamingStrategy (extends DefaultNamingStrategy) preserves the camelCase column name, so the generated SQL is correct.

Why a separate PR

Extracted from the infra-local stack PR (#595) per review — it's a general API fix to the lifecycle crons, unrelated to the local-dev stack.

Verification

Verified against dev RDS (the in-VPC API box): the lifecycle crons run without the TypeORM crash. (No unit test added — a meaningful test requires a live Postgres + full TypeORM entity metadata; this codebase has no integration-test harness for the cron query builders.)

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Fixed ordering behavior in sandbox lifecycle management operations, including auto-stop checks, auto-archive checks, auto-delete checks, and state synchronization to ensure proper sequencing of sandbox operations.

The auto-stop / auto-archive / auto-delete cron queries passed `sandbox."lastBackupAt"` / `activity."lastActivityAt"` to QueryBuilder.orderBy. TypeORM treats the alias.property form and applies its own identifier quoting; the pre-quoted column then gets double-escaped into invalid SQL, crashing the cron. Passing the plain alias.property lets TypeORM map and quote it correctly (CustomNamingStrategy keeps the camelCase column name). Verified against dev RDS (in-VPC API): the lifecycle crons run without the TypeORM crash. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-06-08T04:59:12Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d0998f74-931e-46e1-bd85-1bb601eb8de5

📥 Commits

Reviewing files that changed from the base of the PR and between b302fce and b28c2c9.

📒 Files selected for processing (1)

apps/api/src/sandbox/managers/sandbox.manager.ts

📝 Walkthrough

Walkthrough

This PR removes double-quote wrapping from camelCased column references in TypeORM orderBy expressions across four sandbox lifecycle query builders in sandbox.manager.ts. The changes affect how sandboxes are ordered during auto-stop, auto-archive, auto-delete checks, and state synchronization streaming, without modifying filtering conditions or state-transition logic.

Changes

Sandbox Query OrderBy Syntax

Layer / File(s)	Summary
Remove column reference quotes from orderBy expressions `apps/api/src/sandbox/managers/sandbox.manager.ts`	Four sandbox query builders are updated to use unquoted column references in `orderBy` clauses: `autostopCheck` and `autoArchiveCheck` use `sandbox.lastBackupAt` (instead of `sandbox."lastBackupAt"`), while `autoDeleteCheck` and `syncStates` use `activity.lastActivityAt` (instead of `activity."lastActivityAt"`), affecting the selection and streaming order of sandboxes across lifecycle operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Quotes were bound in SQL so tight,
Now camelCase shines without the blight!
Four queries freed from bracket's hold,
Sandbox order, faster, bold! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely summarizes the main change: removing quotes from orderBy column references in sandbox lifecycle cron queries. It is specific, clear, and directly reflects the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

cla-assistant · 2026-06-08T04:59:17Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

lile seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

cla-assistant · 2026-06-08T04:59:19Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

lile seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates several TypeORM orderBy clauses to remove explicit quoting around camelCase column names.

Changes:

Changed orderBy('sandbox."lastBackupAt"') to orderBy('sandbox.lastBackupAt') in multiple queries.
Changed orderBy('activity."lastActivityAt"') to orderBy('activity.lastActivityAt') in multiple queries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

            .andWhere('sandbox."autoStopInterval" != 0')
            .andWhere('activity."lastActivityAt" < NOW() - INTERVAL \'1 minute\' * sandbox."autoStopInterval"')
-            .orderBy('sandbox."lastBackupAt"', 'ASC')
+            .orderBy('sandbox.lastBackupAt', 'ASC')


        .andWhere('sandbox."autoArchiveInterval" != 0')
        .andWhere('activity."lastActivityAt" < NOW() - INTERVAL \'1 minute\' * sandbox."autoArchiveInterval"')
-        .orderBy('sandbox."lastBackupAt"', 'ASC')
+        .orderBy('sandbox.lastBackupAt', 'ASC')


            .andWhere('sandbox."autoDeleteInterval" >= 0')
            .andWhere('activity."lastActivityAt" < NOW() - INTERVAL \'1 minute\' * sandbox."autoDeleteInterval"')
-            .orderBy('activity."lastActivityAt"', 'ASC')
+            .orderBy('activity.lastActivityAt', 'ASC')


        .andWhere('sandbox."desiredState"::text != sandbox.state::text')
        .andWhere('sandbox."desiredState"::text != :archived', { archived: SandboxDesiredState.ARCHIVED })
-        .orderBy('activity."lastActivityAt"', 'DESC', 'NULLS LAST')
+        .orderBy('activity.lastActivityAt', 'DESC', 'NULLS LAST')


…r infra-local Re-add the OIDC user-create default-region fix that was split to PR boxlite-ai#668. Removing it from this branch broke infra-local: a fresh Dex login creates a Personal org with defaultRegionId=NULL, and dashboard create-sandbox then fails with HTTP 428 "organization does not have a default region" (caught by E2E). Like the runner arm64 (boxlite-ai#671) and cron orderby (boxlite-ai#672) fixes, this is a general fix the local stack depends on, so it stays in-branch until boxlite-ai#668 lands on main; it drops from this PR's diff on the next rebase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

DorianZheng · 2026-06-08T07:00:00Z

i don't think the double-escaping happens, so i'm not sure this fix is needed.

with the installed typeorm (0.3.27), order by isn't quoted at build time — the whole query gets one final pass through replacePropertyNamesForTheWholeQuery. that pass looks up the bare column name (lastBackupAt) in its map. for the already-quoted form sandbox."lastBackupAt", the lookup key becomes "lastBackupAt" (quotes included), misses the map, and the token is left untouched. so it stays valid sandbox."lastBackupAt", not double-quoted.

i replayed typeorm's exact regex (using its own escapeRegExp) on both forms:

sandbox."lastBackupAt" -> sandbox."lastBackupAt" (verbatim, valid)
sandbox.lastBackupAt -> "sandbox"."lastBackupAt" (mapped, valid)

both are valid sql. same story for the activity."lastActivityAt" ones.

corroborating: the where/andWhere clauses in these same queries already use quoted refs (sandbox."autoStopInterval" != 0, activity."lastActivityAt" < ...) and go through the identical final pass — if quoting crashed, those would crash first, and the pr doesn't touch them.

the pr verified the new code runs but didn't show the old code crashing. could the dev RDS crash have come from something else (a join / the stream query) rather than order by? if you still have the failing sql or stack trace, drop it here — that'd settle it. otherwise this looks like a cosmetic no-op.

lilongen · 2026-06-08T10:11:27Z

Closing this — resolving it via dependency pinning instead of an app-source change.

Root cause (thanks @DorianZheng — your skepticism was right): the quoted orderBy is not a no-op-vs-crash issue on the version we actually run. dev/prod build typeorm 0.3.27/0.3.28, where the DISTINCT-subquery pagination trigger is (skip || take) && joinAttributes > 0. The lifecycle crons use .limit(100) (not .take()), so they take the plain path and ORDER BY sandbox."lastBackupAt" is valid SQL — exactly as you predicted. The crash I originally hit was on 0.3.29, which widened that trigger to also include offset || limit, newly routing these queries through createOrderByCombinedWithSelectExpression (which can't resolve the quoted property name → undefined.databaseName).

Why the version differed: apps/yarn.lock is gitignored, so the floating ^0.3.20 range re-resolved per environment — my local picked 0.3.29, CI built 0.3.28.

Fix instead: pin apps/package.json to typeorm@0.3.28, aligning local with prod and stopping the caret from drifting to the regressed 0.3.29. No apps/api change needed.

Note: the column ... does not exist errors currently firing on dev are a separate schema-drift issue, unrelated to orderBy.

The un-quote is still correct hygiene, so if/when we intentionally move to 0.3.29+ we should reapply it first — but that's not needed now.

…sion) apps/yarn.lock is gitignored, so the floating "^0.3.20" range re-resolved per environment: local picked 0.3.29 while dev/prod CI built 0.3.27/0.3.28. typeorm 0.3.29 widened the DISTINCT-subquery pagination trigger in SelectQueryBuilder.executeEntitiesAndRawResults from `(skip || take)` to `(skip || take || offset || limit)`. The sandbox lifecycle crons use innerJoin + quoted orderBy + .limit(100) + getMany, so on 0.3.29 the .limit() now routes them through createOrderByCombinedWithSelectExpression, which fails to resolve the quoted property name and throws `Cannot read properties of undefined (reading 'databaseName')` before any SQL reaches the DB. Pinning to exactly 0.3.28 aligns local with what dev/prod actually run and stops the caret from drifting back to 0.3.29. Do not bump to 0.3.29+ until the quoted-orderBy usage in sandbox.manager.ts is changed to bare property refs (cf. PR boxlite-ai#672). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings June 8, 2026 04:59

Copilot AI reviewed Jun 8, 2026

View reviewed changes

lilongen closed this Jun 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(api): un-quote orderBy column refs in sandbox lifecycle crons#672

fix(api): un-quote orderBy column refs in sandbox lifecycle crons#672
lilongen wants to merge 1 commit into
boxlite-ai:mainfrom
lilongen:fix/sandbox-cron-orderby-unquote

lilongen commented Jun 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

cla-assistant Bot commented Jun 8, 2026

Uh oh!

cla-assistant Bot commented Jun 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

DorianZheng commented Jun 8, 2026

Uh oh!

lilongen commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lilongen commented Jun 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why a separate PR

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

cla-assistant Bot commented Jun 8, 2026

Uh oh!

cla-assistant Bot commented Jun 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

DorianZheng commented Jun 8, 2026

Uh oh!

lilongen commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lilongen commented Jun 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading