Skip to content

fix(tables): heartbeat export job before upload so the stale janitor can't kill a live finalize#5017

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/table-export-finalize-heartbeat
Jun 13, 2026
Merged

fix(tables): heartbeat export job before upload so the stale janitor can't kill a live finalize#5017
waleedlatif1 merged 2 commits into
stagingfrom
fix/table-export-finalize-heartbeat

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Adds an ownership-gate/heartbeat (updateJobProgress) immediately before the export upload — the finalize phase (in-memory join + storage upload) previously emitted no progress updates, so the janitor's stale window was the only bound on slow storage
  • A canceled job now stops before paying for the upload instead of uploading and then deleting the file
  • Fixes the janitor comment to say imports, exports, and deletes are all covered (the UPDATE already applied to all three)
  • Addresses the Cursor Bugbot finding on v0.7.5: deployments API and block, vanta integration, performance improvements, styling consolidation #5016; not exploitable at current bounds (last heartbeat lands right before finalize, threshold is async-timeout+5m, exports are memory-bounded) but the janitor's no-progress-means-stalled invariant shouldn't depend on those constants — self-hosted deploys can shrink the threshold via EXECUTION_TIMEOUT_ASYNC_ENTERPRISE

Type of Change

  • Bug fix

Testing

Added a finalize-gate test (cancel between last page and upload → no upload); all 227 table suite tests pass; typecheck clean

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Jun 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 13, 2026 12:55am

Request Review

@cursor

cursor Bot commented Jun 13, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Targeted export-runner and cron labeling changes; behavior is guarded by existing ownership gates and adds a test for the new finalize gate.

Overview
Adds a finalize ownership heartbeat (updateJobProgress) in runTableExport immediately after row paging finishes and before storage upload. The finalize phase (join chunks + upload) previously did not bump updated_at, so long uploads could look stalled to the stale-table-jobs cron and get marked failed; the heartbeat keeps the janitor’s “no progress means stalled” rule honest without relying on timeout constants.

If the job is canceled or superseded at that gate, the runner throws JobSupersededError and skips upload (and ready/failed transitions), avoiding wasted storage work that would have been deleted afterward.

The stale-execution cron response and comments are aligned with behavior already in place: table jobs cover import, export, and delete, with renamed metrics (tableJobs / staleTableJobsMarkedFailed). A unit test covers cancel between the last page and upload.

Reviewed by Cursor Bugbot for commit f0c50f0. Configure here.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 285950b. Configure here.

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a pre-upload ownership heartbeat to the table export runner so that a canceled or janitor-failed job exits before paying for the expensive storage upload, and renames the misleadingly-named tableImports response key and staleImportsMarkedFailed variable in the cleanup cron to tableJobs / staleTableJobsMarkedFailed to reflect that the stale-job sweep covers imports, exports, and deletes.

  • export-runner.ts: Inserts a updateJobProgress / JobSupersededError gate between the row-accumulation loop and the uploadFile call; a superseded worker now exits cleanly without uploading.
  • export-runner.test.ts: Adds a targeted test that lets the first (loop-entrance) heartbeat pass and fails the finalize gate, asserting no upload, no markJobReady, and no markJobFailed.
  • cleanup-stale-executions/route.ts: Fixes the comment, internal variable name, and JSON response key that previously implied only import jobs were covered by the stale sweep.

Confidence Score: 5/5

Safe to merge — the change adds a cancellation gate before the upload with correct error handling and a targeted test, and the cron rename is a cosmetic-only fix.

The finalize gate inserts two lines that mirror the already-proven loop-entrance pattern; uploadedKey is still null at that point so the catch-block cleanup path is correct. The test accurately models the exact call sequence. The cron renaming touches no logic. No regressions are apparent.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/table/export-runner.ts Adds a finalize-gate heartbeat+cancel check before the upload; error handling correctly leaves uploadedKey as null at that point so no spurious cleanup occurs.
apps/sim/lib/table/export-runner.test.ts New test accurately targets the finalize gate by letting exactly one heartbeat succeed before returning false; assertions on upload/ready/failed mocks are all correct.
apps/sim/app/api/cron/cleanup-stale-executions/route.ts Renames staleImportsMarkedFailed → staleTableJobsMarkedFailed and tableImports → tableJobs throughout; comment also updated — consistent rename, no logic change.

Reviews (2): Last reviewed commit: "chore(tables): rename stale-janitor coun..." | Re-trigger Greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Addressed the 4/5 note: renamed staleImportsMarkedFailedstaleTableJobsMarkedFailed and the tableImports response key → tableJobs to match the corrected comment (the counter always covered imports, exports, and deletes). No consumers read the cron response body, so the key rename is safe.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit f0c50f0. Configure here.

@waleedlatif1 waleedlatif1 merged commit 98948c0 into staging Jun 13, 2026
15 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/table-export-finalize-heartbeat branch June 13, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant