Skip to content

feat(tern): verify re-planned DDL matches reviewed DDL before resume#613

Merged
Kiran01bm merged 3 commits into
mainfrom
kiran01bm/dg-3-apply-time-verify
Jul 1, 2026
Merged

feat(tern): verify re-planned DDL matches reviewed DDL before resume#613
Kiran01bm merged 3 commits into
mainfrom
kiran01bm/dg-3-apply-time-verify

Conversation

@Kiran01bm

Copy link
Copy Markdown
Collaborator

Summary

On resume/recovery, replanAndFilterTasks recomputes each deployment's own delta against its live schema and overwrites each still-needed task's DDL with that recomputed delta before applying it. On a deployment whose schema has drifted, that recomputed delta is DDL no human reviewed — and it was being applied silently. This makes the path fail closed: the resume now refuses to apply DDL that does not match what the task was reviewed with.

What

  • Add verifyReplannedTaskDDL: canonically compare the recomputed DDL against the task's reviewed task.DDL; return a drift error on mismatch. Canonicalization absorbs incidental formatting so only real divergence trips it. A task with no reviewed DDL (legacy synthetic VSchema tasks, already skipped downstream) is left to existing handling.
  • Call it in replanAndFilterTasks before overwriting task.DDL. Both Start() and ResumeApply() callers already propagate the error, so the resume halts for operator reconciliation.
  • Extract formatDriftLocation from formatDriftKey for the operator-facing message.

Why

A drifted non-primary deployment could silently apply a locally recomputed, unreviewed delta on resume. This restores the fail-closed review boundary on the apply/resume path, reusing the shard-aware comparator.

resume re-plan recomputes delta vs live schema
   table still in diff ─▶ canonical(reviewed) == canonical(recomputed)?
                              yes ─▶ apply (unchanged)
                              no  ─▶ drift error → resume STOPS (was: silent apply)
   table not in diff   ─▶ mark completed (unchanged; addressed in follow-up)

Scope: closes the "applies unreviewed DDL" failure mode. The "table dropped out of the diff is silently completed" mode (needs a reviewed-target reference) and shrinking the verify→apply TOCTOU window are tracked as a follow-up.

Copilot AI review requested due to automatic review settings June 30, 2026 22:59

Copilot AI 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.

Pull request overview

This PR strengthens SchemaBot’s fail-closed safety boundary for Tern apply resume/materialization by ensuring that any DDL recomputed against a deployment’s live schema cannot be silently applied unless it canonically matches the DDL that was reviewed and dispatched.

Changes:

  • Added a drift-guard comparison for non-primary plan materialization (verifyMaterializedPlanMatchesLiveSchema) to ensure dispatched reviewed DDL matches this deployment’s locally recomputed plan (shard-aware).
  • Added a resume-time guard (verifyReplannedTaskDDL) to prevent replanAndFilterTasks from overwriting a task’s reviewed DDL with a recomputed (potentially unreviewed) delta.
  • Added targeted tests covering canonicalization tolerance and fail-closed behavior on drift for both materialization and resume.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/tern/local_plan_drift.go Introduces shard-aware multiset comparison + canonicalization helpers for drift detection between reviewed vs recomputed DDL.
pkg/tern/local_plan_drift_guard_test.go Adds unit tests validating the new materialization drift guard behavior (including shard-scoped parity and canonicalization tolerance).
pkg/tern/local_control_resume.go Hooks resume re-plan to verify replanned DDL matches reviewed task DDL before overwriting/continuing.
pkg/tern/local_control_resume_test.go Adds tests ensuring resume fails closed when replanned DDL diverges from reviewed DDL.
pkg/tern/local_client.go Enables drift verification during non-primary plan materialization so unreviewed DDL can’t be replayed on drifted deployments.

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

Comment thread pkg/tern/local_plan_drift.go
Kiran01bm added 2 commits July 1, 2026 10:23
The resume re-plan recomputes each deployment's delta against its live
schema and overwrites task.DDL with it; on a drifted deployment that is
unreviewed DDL applied silently. Fail closed unless the re-plan matches
what the task was reviewed with.
statement.Classify accepts non-DDL (e.g. SELECT, INSERT), which has no
place in a schema-change drift comparison; reject it instead of
canonicalizing it as DDL.
@Kiran01bm Kiran01bm force-pushed the kiran01bm/dg-3-apply-time-verify branch from 173754e to c0d33fe Compare July 1, 2026 00:25
@Kiran01bm Kiran01bm marked this pull request as ready for review July 1, 2026 00:25
@Kiran01bm Kiran01bm requested review from aparajon and morgo as code owners July 1, 2026 00:25
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Kiran01bm Kiran01bm merged commit 13f3887 into main Jul 1, 2026
32 checks passed
@Kiran01bm Kiran01bm deleted the kiran01bm/dg-3-apply-time-verify branch July 1, 2026 01:18
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.

3 participants