improvement(db): contract workspace file sizes - #7128
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Greptile SummaryThis PR completes the workspace-file-size contract after the compatibility rollout.
Confidence Score: 5/5The PR appears safe to merge once its explicitly stated compatibility-release drain condition has been satisfied. The migration validates existing data before enforcing NOT NULL, then removes the compatibility bridge, while changed runtime paths consistently adopt the resulting database contract; no independent blocking or non-blocking defect remains.
|
| Filename | Overview |
|---|---|
| packages/db/migrations/0309_contract_workspace_file_size.sql | Contracts the compatibility schema by validating and enforcing non-null byte sizes before removing the legacy bridge. |
| packages/db/schema.ts | Updates the Drizzle contract to expose only the canonical non-null sizeBytes column. |
| packages/db/script-migrations/index.ts | Removes the completed workspace-file-size backfill from the script-migration registry. |
| .github/workflows/migrations.yml | Removes the temporary development cutover runner after the compatibility phase. |
| apps/sim/ee/workspace-forking/lib/copy/storage-quota.ts | Simplifies fork-copy byte aggregation to rely on the new database non-null invariant. |
| apps/sim/lib/billing/storage/payer-transfer.ts | Removes obsolete missing-size checks from exact storage recomputation while retaining aggregate validation. |
| apps/sim/lib/uploads/shared/types.ts | Tightens the workspace-file-size helper input type to the canonical non-null schema. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Compatibility release] --> B[Backfill size_bytes]
B --> C[Drain legacy writers]
C --> D[Validate size_bytes is non-null]
D --> E[Set column NOT NULL]
E --> F[Drop compatibility trigger and function]
F --> G[Drop legacy size column]
G --> H[Application paths rely on size_bytes]
Reviews (1): Last reviewed commit: "improvement(db): contract workspace file..." | Re-trigger Greptile
There was a problem hiding this comment.
2 issues found across 19 files
Confidence score: 2/5
apps/sim/ee/workspace-forking/lib/copy/storage-quota.tscan undercount copied bytes when selectedworkspace_filesrows havesize_bytes = NULL, allowing quota admission to exceed the intended limit — handle NULL sizes before summing or admitting the copy.packages/db/migrations/0309_contract_workspace_file_size.sqlcan abort on existing databases containing legacy rows with NULLsize_bytes, blocking migration — add an idempotent backfill from the still-present non-nullsizecolumn before validation.
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="apps/sim/ee/workspace-forking/lib/copy/storage-quota.ts">
<violation number="1" location="apps/sim/ee/workspace-forking/lib/copy/storage-quota.ts:51">
P1: When a selected `workspace_files` row still has `size_bytes = NULL`, PostgreSQL ignores it in `sum`, and this `coalesce` turns an all-NULL selection into zero, allowing quota admission to undercount copied bytes. Preserve the NULL-detection aggregate and make the 503 path handle a null total.
(Based on your team's feedback about failing closed on missing `size_bytes`.) [3f8e6e9d-39ca-4a4a-b8b4-0e6be9b461e0].</violation>
</file>
<file name="packages/db/migrations/0309_contract_workspace_file_size.sql">
<violation number="1" location="packages/db/migrations/0309_contract_workspace_file_size.sql:5">
P1: When an existing database has rows from before `size_bytes` was added, this validation aborts because the migration does not backfill NULLs. Add an idempotent update from the still-present non-null `size` column before validation so self-hosted upgrades do not depend on the removed runner.
(Based on your team's feedback about deployable schema/data migrations.)</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| ELSE coalesce(sum(${workspaceFiles.sizeBytes}), 0) | ||
| END | ||
| : sql<number>`( | ||
| SELECT coalesce(sum(${workspaceFiles.sizeBytes}), 0) |
There was a problem hiding this comment.
P1: When a selected workspace_files row still has size_bytes = NULL, PostgreSQL ignores it in sum, and this coalesce turns an all-NULL selection into zero, allowing quota admission to undercount copied bytes. Preserve the NULL-detection aggregate and make the 503 path handle a null total.
(Based on your team's feedback about failing closed on missing size_bytes.) [3f8e6e9d-39ca-4a4a-b8b4-0e6be9b461e0].
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/ee/workspace-forking/lib/copy/storage-quota.ts, line 51:
<comment>When a selected `workspace_files` row still has `size_bytes = NULL`, PostgreSQL ignores it in `sum`, and this `coalesce` turns an all-NULL selection into zero, allowing quota admission to undercount copied bytes. Preserve the NULL-detection aggregate and make the 503 path handle a null total.
(Based on your team's feedback about failing closed on missing `size_bytes`.) [3f8e6e9d-39ca-4a4a-b8b4-0e6be9b461e0].</comment>
<file context>
@@ -47,11 +47,8 @@ export async function sumForkCopyBytes(
- ELSE coalesce(sum(${workspaceFiles.sizeBytes}), 0)
- END
+ : sql<number>`(
+ SELECT coalesce(sum(${workspaceFiles.sizeBytes}), 0)
FROM ${workspaceFiles}
WHERE ${and(
</file context>
| VALIDATE CONSTRAINT "workspace_files_size_bytes_not_null_check";--> statement-breakpoint | ||
| -- migration-safe: contract of #7112 and #7123 — application reads and writes use size_bytes, the backfill is complete, and this PR must merge only after the compatibility release fully drains |
There was a problem hiding this comment.
P1: When an existing database has rows from before size_bytes was added, this validation aborts because the migration does not backfill NULLs. Add an idempotent update from the still-present non-null size column before validation so self-hosted upgrades do not depend on the removed runner.
(Based on your team's feedback about deployable schema/data migrations.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/migrations/0309_contract_workspace_file_size.sql, line 5:
<comment>When an existing database has rows from before `size_bytes` was added, this validation aborts because the migration does not backfill NULLs. Add an idempotent update from the still-present non-null `size` column before validation so self-hosted upgrades do not depend on the removed runner.
(Based on your team's feedback about deployable schema/data migrations.) </comment>
<file context>
@@ -0,0 +1,14 @@
+ ADD CONSTRAINT "workspace_files_size_bytes_not_null_check"
+ CHECK ("size_bytes" IS NOT NULL) NOT VALID;--> statement-breakpoint
+ALTER TABLE "workspace_files"
+ VALIDATE CONSTRAINT "workspace_files_size_bytes_not_null_check";--> statement-breakpoint
+-- migration-safe: contract of #7112 and #7123 — application reads and writes use size_bytes, the backfill is complete, and this PR must merge only after the compatibility release fully drains
+ALTER TABLE "workspace_files" ALTER COLUMN "size_bytes" SET NOT NULL;--> statement-breakpoint
</file context>
| VALIDATE CONSTRAINT "workspace_files_size_bytes_not_null_check";--> statement-breakpoint | |
| -- migration-safe: contract of #7112 and #7123 — application reads and writes use size_bytes, the backfill is complete, and this PR must merge only after the compatibility release fully drains | |
| UPDATE "workspace_files" | |
| SET "size_bytes" = "size" | |
| WHERE "size_bytes" IS NULL;--> statement-breakpoint | |
| ALTER TABLE "workspace_files" | |
| VALIDATE CONSTRAINT "workspace_files_size_bytes_not_null_check";--> statement-breakpoint |
Summary
workspace_files.size_bytesnon-null and remove the legacysizebridge, trigger, and functionType of Change
Testing
bun run lintbun run check:auditsbun run type-checkbun run check:migrations origin/stagingbunx drizzle-kit checkChecklist