-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: rescue tombstoned prod tables #17687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@Dakuan I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
c90647a to
2224c5c
Compare
adrinr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works fine, I managed to reproduce he issue locally. I am just worried about the performance of it. Instead of relying on the rows, that can be millions, we should probably compare the list of tables in dev vs the ones in prod. Otherwise publishing workspaces with a lot of rows will cause delays
|
@Dakuan I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files
Confidence score: 3/5
- The unreachable
if (prodTable._deleted)branch inpackages/server/src/api/controllers/deploy/index.tsmeans the expected revision for tombstoned tables is never captured, so deploy logic can’t handle deleted documents correctly. - This gap could surface as failed or inconsistent deploys when production documents were previously deleted, giving the change a meaningful regression risk.
- Pay close attention to
packages/server/src/api/controllers/deploy/index.ts- confirm tombstoned documents are handled without relying on an unreachable_deletedbranch.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/server/src/api/controllers/deploy/index.ts">
<violation number="1" location="packages/server/src/api/controllers/deploy/index.ts:306">
P1: The `if (prodTable._deleted)` branch is unreachable. CouchDB returns 404 for tombstoned documents, so `tryGet` returns `undefined` rather than a doc with `_deleted: true`. The `prodRev` will never be captured for tombstoned tables, which may cause conflict errors when putting the rescued doc. Consider using `allDocs` with `keys: [tableId]` which can return deleted doc metadata, or fetch with explicit revision.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| targets.push({ tableId }) | ||
| continue | ||
| } | ||
| if (prodTable._deleted) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: The if (prodTable._deleted) branch is unreachable. CouchDB returns 404 for tombstoned documents, so tryGet returns undefined rather than a doc with _deleted: true. The prodRev will never be captured for tombstoned tables, which may cause conflict errors when putting the rescued doc. Consider using allDocs with keys: [tableId] which can return deleted doc metadata, or fetch with explicit revision.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/src/api/controllers/deploy/index.ts, line 306:
<comment>The `if (prodTable._deleted)` branch is unreachable. CouchDB returns 404 for tombstoned documents, so `tryGet` returns `undefined` rather than a doc with `_deleted: true`. The `prodRev` will never be captured for tombstoned tables, which may cause conflict errors when putting the rescued doc. Consider using `allDocs` with `keys: [tableId]` which can return deleted doc metadata, or fetch with explicit revision.</comment>
<file context>
@@ -185,6 +187,168 @@ async function applyPendingColumnRenames(
+ targets.push({ tableId })
+ continue
+ }
+ if (prodTable._deleted) {
+ targets.push({ tableId, prodRev: prodTable._rev })
+ }
</file context>
Summary by cubic
Rescues tombstoned prod table docs during publish by restoring table metadata when prod has live rows, preventing broken tables and data loss.
Written for commit b9345a2. Summary will update on new commits.