Skip to content

fix(server): migrate legacy thread pull request links - #19129

Open
Yash-Singh1 wants to merge 4 commits into
mainfrom
fix/server-migrate-thread-pull-request-links
Open

Yash-Singh1 wants to merge 4 commits into
mainfrom
fix/server-migrate-thread-pull-request-links

Conversation

@Yash-Singh1

@Yash-Singh1 Yash-Singh1 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Linked-thread lookup scans the old linkedPullRequest field on every request to support older V2 thread records.

Change

Add migration 63 to convert those links to pullRequests in thread projections and V2 source events, so projection rebuilds preserve the conversion. Keep existing arrays, including explicit empty arrays, and leave V1 events unchanged. Initialize new threads with an empty array and remove the legacy query branch.

Scope and approval

This is the maintainer-requested migration of an existing V2 storage format and removal of its redundant query fallback. It preserves existing pull-request behavior and the V1 transition. Macroscope approved this scope at e971cf0: #19129 (review).

The branch includes current main. The earlier snooze and queue fixes already landed in #19099 and are outside this PR's remaining diff.

Verification

After merging main and renumbering the migration, all 49 focused tests passed, including migration pagination, projection rebuilds, linked-thread lookup, V1 importing, Stop, and recovery. Server typecheck, targeted lint, and formatting checks passed. The previous description's statement that tests were not run is obsolete.

CI and the thread-transfer performance report also pass on e971cf0.

Model and harness: GPT 6.1 Sol, Codex.

- Keep notification and delegated completion runs queued when snoozing.
- Cancel buffered provider wakes on Stop and runtime recovery.
- Read only queued message records when checking buffered wakes
- Keep held queues out of recovery scans
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 13, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at e971cf0

Macroscope's review found this PR approvable — This is a focused backward-compatibility fix that canonicalizes legacy pull-request links, preserves existing arrays and V1 data, and removes a redundant lookup path. Its startup migration is bounded, paginated, and covered by targeted tests without introducing a new capability or sensitive behavior.

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

@github-actions

github-actions Bot commented Oct 13, 2026 •

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 +23 B (+0.5%) 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 +23 B (+1.9%) 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 −7 B (−0.1%) 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 −7 B (−0.6%) 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: 339179f · PR result: e971cf0 · 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.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 8185f17b-74fa-4d9a-9c10-5244f6832e93
📥 Commits

Reviewing files that changed from the base of the PR and between bc7d75c and e971cf0.

📒 Files selected for processing: 6
  • apps/server/src/orchestration-v2/Orchestrator.ts
  • apps/server/src/persistence/Migrations.ts
  • apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts
  • apps/server/src/persistence/Migrations/063_CanonicalizeThreadPullRequestLinks.test.ts
  • apps/server/src/persistence/Migrations/063_CanonicalizeThreadPullRequestLinks.ts
  • apps/server/src/persistence/reconcileV2PreviewMigration.test.ts

Included review availability: This review used your included allowance. 9 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

📝 Walkthrough

Migration 63 canonicalizes legacy thread pull-request links in projections and version-2 events. New threads initialize pullRequests as an empty array. Linked-thread lookup now reads only canonical pull-request arrays.

Changes: Pull Request Link Canonicalization

Layer / File(s) Summary
Migration and validation
apps/server/src/persistence/Migrations/063_CanonicalizeThreadPullRequestLinks.ts, apps/server/src/persistence/Migrations.ts, apps/server/src/persistence/Migrations/063_CanonicalizeThreadPullRequestLinks.test.ts, apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts, apps/server/src/persistence/reconcileV2PreviewMigration.test.ts
Migration 63 converts qualifying legacy links in projection and version-2 event payloads in batches. Registration and migration expectations include migration 63. Tests cover conversion, pagination, timestamps, retries, and projection rebuilding.
Canonical link initialization and lookup
apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/pullRequest/linkedThreads.ts, apps/server/src/pullRequest/linkedThreads.test.ts
New threads start with pullRequests: []. Linked-thread lookup removes the legacy payload fallback and compares requested keys against canonical array entries.

Priority: ⬇️ Low

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Migration63
  participant ThreadProjections
  participant Version2Events
  participant LinkedThreadLookup
  Migration63->>ThreadProjections: Canonicalize legacy linkedPullRequest values
  Migration63->>Version2Events: Canonicalize version-2 event payloads
  ThreadProjections->>LinkedThreadLookup: Provide canonical pullRequests arrays
  LinkedThreadLookup->>LinkedThreadLookup: Match requested pull-request keys
Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to e971c

No actionable merge-blocking issue remains; the canonical-link migration is mergeable after normal checks.

Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title is concise, uses the repository's conventional format, and accurately describes the migration of legacy thread pull request links.
Description check Passed The description includes all required sections. It explains the problem, the migration approach, scope approval, focused verification results, and the model and harness used.
✨ 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.

This branch has not been deployed

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

Labels

size:L 100-499 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.

1 participant