Skip to content

fix(github): report a failed apply's furthest copy progress and stop claiming unverified checkpoint resumes - #1110

Draft
aparajon wants to merge 1 commit into
mainfrom
claude/brave-morse-4fdd18
Draft

fix(github): report a failed apply's furthest copy progress and stop claiming unverified checkpoint resumes#1110
aparajon wants to merge 1 commit into
mainfrom
claude/brave-morse-4fdd18

Conversation

@aparajon

@aparajon aparajon commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Two operator-facing accuracy fixes for applies that went through operator retries:

  • "Failed at N%" now reports the run's furthest copy progress. Each retry relaunches the engine's row copy, so the live per-table progress resets with every attempt — the failure label was showing whichever attempt was sampled last, which can understate a run whose earlier attempt got further (or read as a completed copy when an earlier attempt hit 100%). The tasks table gains best_rows_copied / best_progress_percent high-water columns (both dialects), raised by the store on every progress write and never allowed to regress; the terminal summary label and the in-progress failed row's bar render from the high-water. A storage parity test pins the never-regress contract on MySQL and PostgreSQL.
attempt 1 copy ──────────────▶ 47% ✗ interrupted
attempt 2 copy ────▶ 12% ✗ failed (sampled last)

live progress:   12%   ── what the comment used to say
high-water:      47%   ── what it says now
  • The driver no longer claims "resumed from checkpoint" before the engine confirms it. The resume timeline event named the code path, not the outcome — it claimed a checkpoint resume even when the engine found no checkpoint and restarted the copy from scratch. The relaunch event now says only that the apply resumed; the checkpoint claim comes solely from the engine-confirmed resume event, and the engine's own "could not resume from checkpoint" log line covers the restart case. The deferred-cutover recovery event now names what was actually verified (the engine cutover signal).
Summary: Failed (Retries Exhausted) — the failure label reports the run's high-water (47%), not the last-sampled attempt (12%)

❌ Schema Change Failed — Staging

Database: testapp | Apply ID: apply-a1b2c3d4e5f6 | Duration: 2h 15m

Applied by @jackjackbits at 2026-03-15 12:15:00 UTC

⚠️ Error: lock wait timeout exceeded; try restarting transaction

1 of 2 tables completed before failure.

users — Failed at 47%

ALTER TABLE `users` ADD INDEX `idx_email`(`email`);

orders

ALTER TABLE `orders` ADD INDEX `idx_user_id`(`user_id`);

To retry:

schemabot apply -e staging

Note

Deployed PostgreSQL storage databases need the two new tasks columns added by hand before this deploys (PostgreSQL bootstrap verifies columns but does not alter existing tables).

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 22, 2026 14:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates SchemaBot’s GitHub-facing rendering and timeline events so operator-visible surfaces don’t claim unverified checkpoint resumes and don’t report misleading row-copy progress when an apply has been retried (where the stored per-table percent can reflect an arbitrary attempt).

Changes:

  • Omit progress bars / “Failed at N%” labels on failed rows and summary entries when Attempt > 0 (the apply has been redispatched at least once), while keeping the percentage for single-attempt failures.
  • Adjust LocalClient resume/recovery timeline messages to avoid claiming “resumed from checkpoint” unless the engine confirms it, and add coverage to enforce that invariant.
  • Add a new preview type + template snippet for “Summary: Failed (Retries Exhausted)” and corresponding rendering/tests.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TEMPLATES.md Adds a rendered template example for “Summary: Failed (Retries Exhausted)”.
pkg/webhook/templates/preview.go Adds a preview renderer for the retry-exhausted failed summary.
pkg/webhook/templates/apply.go Suppresses failed-row progress bars and “Failed at N%” in multi-attempt runs; threads multi-attempt flag into summary table entries.
pkg/webhook/templates/apply_test.go Adds tests ensuring multi-attempt failures omit the bar/percent while single-attempt failures retain them.
pkg/tern/local_control_resume.go Updates relaunch/recovery timeline event messages to avoid unverified “checkpoint” claims.
pkg/tern/local_control_resume_test.go Adds a test asserting relaunch events do not contain “checkpoint” unless engine-confirmed.
pkg/cmd/internal/templates/preview.go Registers a new preview type for “failed after retries exhausted”.
pkg/cmd/internal/templates/preview_dispatch.go Wires the new preview type to the webhook template preview renderer.
pkg/cmd/internal/templates/preview_comment.go Includes the new summary preview in the “all previews” outputs.
pkg/cmd/commands/preview.go Adds the new preview type to the CLI’s allowed preview set.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…claiming unverified checkpoint resumes

Two operator-facing accuracy fixes for applies that consumed operator
retries:

1. "Failed at N%" now reports the run's high-water row-copy progress
   instead of whichever attempt the control plane happened to sample
   last. Each retry relaunches the engine's row copy, so the live
   per-table progress resets with every attempt; a label built from it
   understates a run whose earlier attempt got further, and a stale
   "Failed at 100%" from an earlier attempt reads as a completed copy.
   The tasks table gains best_rows_copied / best_progress_percent
   columns (both dialects), raised by the store on every progress write
   and never allowed to regress; the terminal summary label and the
   in-progress failed row's bar render from the high-water. A parity
   test pins the never-regress contract on MySQL and PostgreSQL, and a
   TEMPLATES.md preview shows the retries-exhausted summary.

2. The driver's resume timeline events no longer claim "resumed from
   checkpoint" before the engine has confirmed it. The relaunch event
   now says only that the apply resumed; the checkpoint claim comes
   solely from the engine-confirmed resume event, and the engine's own
   "could not resume from checkpoint" log line covers the restart case.
   The deferred-cutover recovery event now names what was actually
   verified (the engine cutover signal).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@aparajon
aparajon force-pushed the claude/brave-morse-4fdd18 branch from 26e0737 to 485a178 Compare August 23, 2026 03:55
@aparajon aparajon changed the title fix(github): stop reporting stale progress and unverified checkpoint resume on retried applies fix(github): report a failed apply's furthest copy progress and stop claiming unverified checkpoint resumes Aug 23, 2026
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.

2 participants