Skip to content

fix(server): upgrade preview databases with peer-link migration collisions - #25293

Closed
juliusmarminge wants to merge 3 commits into
mainfrom
fix/issue-25289
Closed

juliusmarminge wants to merge 3 commits into
mainfrom
fix/issue-25289

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Problem

Preview 3511/3516 databases record peer-link migrations at 68–70. Starting current main against that history fails before the server listens because required migrations ShellStopRequestIndex and ThreadMessageSearch would be skipped.

Change

Recognize this exact preview history and, in one transaction, move its peer-link migration records to 70–72 and apply the missing main migrations 68/69. Preserve the existing peer-link schema and data without replaying its DDL. Leave unrecognized histories subject to the existing migration guard.

Scope and approval

Fixes #25289

Triaged bug explicitly enqueued by Julius via the automated issue flow: #25289.

Verification

  • Red regression failed on main with the reported migration collision; green regression preserves a fenced peer-link row and its waiting-link index and verifies a second run changes nothing. A near-match history is rejected without mutation.
  • After merging current main, 17 tests passed across Migrations.test.ts, reconcileV2PreviewMigration.test.ts, and initializeV2Database.test.ts.
  • vp run check:changed: format, lint, GraphQL, knip, affected typechecks and adjacent tests passed.
  • Real runtime reproduction used an isolated SQLite backup from a read-only source. Before: snapshot migration and dev-server startup failed with the collision. After: snapshot migration applied 68/69; the pruned database started successfully and completed orchestration recovery with all counts zero. Live userdata was never modified.
  • Sanitized red/before/after runtime transcript and schema verification. Runtime evidence was captured at the fix commit; the subsequent main merge did not change the three migration/test files.
  • Both independent reviews found no blockers; proof audit passed. Server-only change: visual capture matrix is empty. Main still logs the existing warning for peer-link records 70–72, whose implementations are only on preview; the repaired numbering matches preview 3702.

Models: Claude Opus 5.5 and GPT-6.1, through T3 Code's Claude Agent and Codex harnesses.

juliusmarminge and others added 3 commits October 26, 2026 03:49
Refs #25289

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Previews 3511/3516 recorded PeerLinks, PeerLinkFence and
WaitingLinkRequestIndex at ids 68-70, which main uses for
ShellStopRequestIndex and ThreadMessageSearch, so startup refused the
history. Recognize that exact history, renumber the peer-link rows to
70-72 (the ids 3702 uses) keeping their schema and data, and apply 68/69
in the same transaction. Any other history still hits the guard.

Fixes #25289

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 26, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b8c5346

Macroscope's review found this PR approvable — This is a focused migration compatibility fix that repairs only recognized preview histories while leaving current and unknown database histories protected by existing guards. The included tests cover upgrade behavior, preservation of peer-link data, idempotence, and refusal of unexpected histories.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c3888da and b8c5346.

Files selected for processing: 3
  • apps/server/src/persistence/Migrations.test.ts
  • apps/server/src/persistence/Migrations.ts
  • apps/server/src/persistence/reconcilePeerLinkPreviewMigration.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

The migration runner now reconciles a specific peer-link preview migration history before regular migrations. Tests cover successful upgrades, preservation of peer-link data and indexes, repeated runs, and rejection of a history that does not match the expected sequence.

Changes: Peer-link migration upgrade

Layer / File(s) Summary
Reconcile preview migration history
apps/server/src/persistence/reconcilePeerLinkPreviewMigration.ts, apps/server/src/persistence/Migrations.ts, apps/server/src/persistence/Migrations.test.ts
The reconciliation checks whether migration history matches the expected preview sequence. On a match, it shifts preview records to IDs 70–72, applies migrations 68 and 69, and returns their records to the runner. The runner skips reconciliation when the inclusive migration limit is below 69. Tests verify the upgrade, preserved data and index, no-op repeat run, and rejection of a mismatched history.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

Suggested labels: size:M

Suggested reviewers: t3dotgg, macroscopeapp

Merge Risk: Minimal · up to b8c53

The exact preview database history has a transactional upgrade path, with no identified issue requiring a fix before merge.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title is concise, uses conventional commit format, and clearly describes the server fix for preview database migration collisions.
Description check Passed The description includes all required sections. It explains the problem, implementation, scope and approval, focused verification, runtime validation, and agent details.
Linked Issues check Passed Issue #25289 requires recognition of the exact 68–70 peer-link history, relocation to 70–72, and application of migrations 68 and 69 without replaying peer-link DDL. `reconcilePeerLinkPreviewMigration…
Out of Scope Changes check Passed The changed files are limited to the migration integration, the focused peer-link history reconciliation, and regression tests. The reconciliation and tests directly support issue #25289. No unrelated…
Full details: Linked Issues check

Explanation

Issue #25289 requires recognition of the exact 68–70 peer-link history, relocation to 70–72, and application of migrations 68 and 69 without replaying peer-link DDL. reconcilePeerLinkPreviewMigration matches IDs 55–70 exactly, updates peer-link records in reverse order, runs ShellStopRequestIndex and ThreadMessageSearch in one transaction, and inserts their history records. Migrations.ts invokes this reconciliation before the regular loader. Tests verify data and the waiting-link index remain, the second run is a no-op, and a near-match is rejected without mutation.

Full details: Out of Scope Changes check

Explanation

The changed files are limited to the migration integration, the focused peer-link history reconciliation, and regression tests. The reconciliation and tests directly support issue #25289. No unrelated product behavior or unrelated files are shown in the reviewed changes.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 4.9 KiB 5.0 KiB +30 B (+0.6%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB +30 B (+2.5%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.8 KiB 20.9 KiB +41 B (+0.2%) 29.3 KiB ✅
Codex Live turn messages 1 2 +1 (+100.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: c3888da · PR result: b8c5346 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preview 3702 cannot start on a 3511/3516 database: peer-link migrations moved from 68–70 to 70–72

1 participant