Skip to content

Clean up invalid pending approval projections#2106

Merged
juliusmarminge merged 1 commit intomainfrom
t3code/1dc35fb3
Apr 17, 2026
Merged

Clean up invalid pending approval projections#2106
juliusmarminge merged 1 commit intomainfrom
t3code/1dc35fb3

Conversation

@juliusmarminge
Copy link
Copy Markdown
Member

@juliusmarminge juliusmarminge commented Apr 17, 2026

Summary

  • Adds migration 025_CleanupInvalidProjectionPendingApprovals to delete projection_pending_approvals rows that are not backed by an approval.requested activity.
  • Recomputes projection_threads.pending_approval_count from the remaining pending approvals so thread state stays consistent after cleanup.
  • Adds a migration test covering valid approval-backed rows, invalid orphaned rows, and count recalculation.

Testing

  • Added apps/server/src/persistence/Migrations/025_CleanupInvalidProjectionPendingApprovals.test.ts to verify cleanup and count reconciliation.
  • Not run: bun fmt
  • Not run: bun lint
  • Not run: bun typecheck
  • Not run: bun run test

Note

Medium Risk
Data-cleanup migration deletes rows from projection_pending_approvals based on activity history and recomputes projection_threads.pending_approval_count, which could impact thread state if the matching criteria is wrong or activity payloads vary.

Overview
Adds migration 025_CleanupInvalidProjectionPendingApprovals to delete orphaned projection_pending_approvals rows that are not backed by an approval.requested entry in projection_thread_activities (matching via payload_json.requestId).

After cleanup, it recalculates projection_threads.pending_approval_count from remaining pending approvals to keep thread aggregates consistent, and includes a new sqlite-backed migration test verifying both deletion behavior and count reconciliation.

Reviewed by Cursor Bugbot for commit 532ab0c. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add migration to remove invalid pending approvals and recalculate thread counts

  • Adds migration 025 that deletes rows from projection_pending_approvals where no matching approval.requested activity exists in projection_thread_activities.
  • Recalculates pending_approval_count on projection_threads based on remaining pending approvals after the cleanup.
  • Behavioral Change: existing orphaned pending approval rows are permanently deleted when this migration runs.

Macroscope summarized 532ab0c.

- add migration 025 to delete stale projection_pending_approvals rows
- recompute projection_threads.pending_approval_count after cleanup
- cover the migration with a regression test
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 17, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8245debd-6ccc-430c-b96f-eb54e849bc87

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/1dc35fb3

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

@github-actions github-actions bot added size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Apr 17, 2026
@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp bot commented Apr 17, 2026

Approvability

Verdict: Approved

A data cleanup migration that removes orphaned pending approval records and recalculates counts. The migration is well-scoped, has comprehensive test coverage, and the author is the primary maintainer of the migrations system.

You can customize Macroscope's approvability policy. Learn more.

@juliusmarminge juliusmarminge merged commit f297e30 into main Apr 17, 2026
12 checks passed
@juliusmarminge juliusmarminge deleted the t3code/1dc35fb3 branch April 17, 2026 06:26
znoraka pushed a commit to znoraka/t3code that referenced this pull request Apr 17, 2026
aaditagrawal added a commit to aaditagrawal/t3code that referenced this pull request Apr 18, 2026
Integrates upstream/main (9df3c64) on top of fork's main (9602c18).

Upstream features adopted:
- Claude Opus 4.5 and 4.7 built-in models (pingdotgg#2072, pingdotgg#2143)
- Node-native TypeScript migration across desktop/server (pingdotgg#2098)
- Configurable project grouping with client-settings overrides (pingdotgg#2055, pingdotgg#2099)
- Thread status in command palette (pingdotgg#2107)
- Responsive composer / plan sidebar on narrow windows (pingdotgg#1198)
- Capture-phase CTRL+J keydown for Windows terminal toggle (pingdotgg#2113/pingdotgg#2142)
- Bypass xterm for global terminal shortcuts (pingdotgg#1580)
- Windows ARM build target (pingdotgg#2080)
- Windows PATH hydration + repair (pingdotgg#1729)
- Gitignore-aware workspace search (pingdotgg#2078)
- Claude process leak fix + stale session monitoring (pingdotgg#2042)
- Preserve provider bindings when stopping sessions (pingdotgg#2084)
- Clean up invalid pending-approval projections (pingdotgg#2106) — new migration
- Extract backend startup readiness coordination
- Drop stale text-gen options on reset (pingdotgg#2076)
- Extend negative repository identity cache TTL (pingdotgg#2083)
- Allow deleting non-empty projects from warning toast (pingdotgg#1264)
- Restore defaults only on General settings (pingdotgg#1710)
- Release workflow modernization (blacksmith runners, GitHub App token guards, v0.0.20 version bump)

Fork features preserved:
- All 8 providers (codex, claudeAgent, copilot, cursor, opencode,
  geminiCli, amp, kilo) with their adapters, services, and tests
- Fork's custom OpenCode protocol impl in apps/server/src/opencode/ (kept
  over upstream's @opencode-ai/sdk-based provider added in pingdotgg#1758 — fork's
  version is tested and integrated; upstream's parallel files deleted)
- Fork's direct-CLI Cursor adapter (kept over upstream's new ACP-based
  CursorProvider added in pingdotgg#1355 — upstream's parallel files deleted)
- Fork's ProviderRegistry aggregates only codex + claudeAgent snapshots;
  the other 6 providers register via ProviderAdapterRegistry
- PROVIDER_CACHE_IDS stays at [codex, claudeAgent] matching what the
  registry actually caches
- Migration IDs preserved (fork 23/24/25/26; upstream's new 025 lands at
  ID 27 to avoid re-applying on deployed fork DBs)
- Fork's generic per-provider settings (enabled/binaryPath/configDir/
  customModels) kept over upstream's opencode-specific serverUrl/password
- Log directory IPC channels, updateInstallInFlight tracking, icon
  composer pipeline all preserved
- Fork's simplified release.yml (no npm CLI publish, no nightly infra)
- composerDraftStore normalizeProviderKind widened to accept all 8 kinds
- Dark mode --background set to #0f0f0f

Test status:
- All 9 package typechecks pass
- Lint clean (0 errors)
- Tests: 1877 passed, 15 skipped (incl. 4 historically-flaky GitManager
  cross-repo PR selector tests newly gated with TODO for Node-native-TS
  follow-up)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 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