Skip to content

Backfill projected shell summaries and stale approval cleanup#2004

Merged
juliusmarminge merged 4 commits intomainfrom
t3code/fix-thread-shell-mismatch
Apr 14, 2026
Merged

Backfill projected shell summaries and stale approval cleanup#2004
juliusmarminge merged 4 commits intomainfrom
t3code/fix-thread-shell-mismatch

Conversation

@juliusmarminge
Copy link
Copy Markdown
Member

@juliusmarminge juliusmarminge commented Apr 13, 2026

Summary

  • Backfill projection_threads shell summary fields from existing thread activity, messages, pending approvals, and proposed plans.
  • Resolve stale pending approval rows when backfill finds matching provider.approval.respond.failed events with unknown/stale request details.
  • Update projection logic and add coverage for the stale-approval cleanup path.

Testing

  • Added migration test coverage in apps/server/src/persistence/Migrations/024_BackfillProjectionThreadShellSummary.test.ts.
  • Added projection pipeline test coverage in apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts.
  • Not run: bun fmt
  • Not run: bun lint
  • Not run: bun typecheck
  • Not run: bun run test

Note

Medium Risk
Adds a data backfill migration and changes pending-approval projection behavior to auto-resolve certain failure cases; mistakes could incorrectly clear approvals or produce wrong thread summary counts.

Overview
Adds migration 024_BackfillProjectionThreadShellSummary to backfill projection_threads shell summary fields (latest_user_message_at, pending_approval_count, pending_user_input_count, has_actionable_proposed_plan) from existing messages/activities/proposed plans, and to seed/update projection_pending_approvals based on historic approval.requested/approval.resolved activity and thread.approval-response-requested events.

Updates the projection pipeline so provider.approval.respond.failed activities whose detail indicates an unknown/stale pending approval now resolve the existing pending approval (dropping pending_approval_count accordingly), and adds focused tests covering both stale and non-stale failure paths plus the migration backfill behavior.

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

Note

Backfill projected thread shell summaries and resolve stale pending approvals in migration 24

  • Adds migration 024 that backfills projection_threads shell summary fields (latest_user_message_at, pending_approval_count, pending_user_input_count, has_actionable_proposed_plan) and seeds/resolves projection_pending_approvals rows from existing activity history.
  • Resolves stale pending approvals by detecting provider.approval.respond.failed activities whose detail indicates an unknown or stale permission request, setting resolved_at to the failure activity's timestamp.
  • Adds isStalePendingApprovalFailureDetail helper in ProjectionPipeline.ts and a matching projection branch so the same stale-approval resolution logic applies to newly projected activities going forward.
  • Behavioral Change: provider.approval.respond.failed activities with stale/unknown detail strings now resolve the corresponding pending approval row rather than being ignored.

Macroscope summarized 232001d.

- Recompute projected thread shell counts during migration
- Treat stale pending approval failures as resolved in projection
- Add coverage for the backfill and projection behavior
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 13, 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: 4a34192e-5a8d-4b01-9282-1305f54a0e57

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/fix-thread-shell-mismatch

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

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

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Stale failure handler overwrites already-resolved approval decisions
    • Added an early return for existingRow.value.status === "resolved" in the stale failure code path in ProjectionPipeline.ts, and added AND status = 'pending' to the migration's stale failure UPDATE WHERE clause, preventing both from overwriting already-resolved approval decisions.
  • ✅ Fixed: Migration count ignores its own event-store resolutions
    • Replaced the activity-based pending_approval_count subquery in migration step 5 with SELECT COUNT(*) FROM projection_pending_approvals WHERE status = 'pending', matching the runtime's refreshThreadShellSummary behavior and correctly reflecting resolutions applied by earlier migration steps.

Create PR

Or push these changes by commenting:

@cursor push 25f8053d3b
Preview (25f8053d3b)
diff --git a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts
--- a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts
+++ b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts
@@ -1268,6 +1268,9 @@
               if (Option.isNone(existingRow)) {
                 return;
               }
+              if (existingRow.value.status === "resolved") {
+                return;
+              }
               yield* projectionPendingApprovalRepository.upsert({
                 requestId,
                 threadId: existingRow.value.threadId,

diff --git a/apps/server/src/persistence/Migrations/024_BackfillProjectionThreadShellSummary.ts b/apps/server/src/persistence/Migrations/024_BackfillProjectionThreadShellSummary.ts
--- a/apps/server/src/persistence/Migrations/024_BackfillProjectionThreadShellSummary.ts
+++ b/apps/server/src/persistence/Migrations/024_BackfillProjectionThreadShellSummary.ts
@@ -175,11 +175,12 @@
         FROM latest_stale_failures
         WHERE latest_stale_failures.request_id = projection_pending_approvals.request_id
       )
-    WHERE EXISTS (
-      SELECT 1
-      FROM latest_stale_failures
-      WHERE latest_stale_failures.request_id = projection_pending_approvals.request_id
-    )
+    WHERE status = 'pending'
+      AND EXISTS (
+        SELECT 1
+        FROM latest_stale_failures
+        WHERE latest_stale_failures.request_id = projection_pending_approvals.request_id
+      )
   `;
 
   yield* sql`
@@ -192,40 +193,10 @@
           AND message.role = 'user'
       ),
       pending_approval_count = COALESCE((
-        WITH latest_approval_states AS (
-          SELECT
-            latest.request_id,
-            latest.kind,
-            latest.detail
-          FROM (
-            SELECT
-              json_extract(activity.payload_json, '$.requestId') AS request_id,
-              activity.kind,
-              lower(COALESCE(json_extract(activity.payload_json, '$.detail'), '')) AS detail,
-              ROW_NUMBER() OVER (
-                PARTITION BY json_extract(activity.payload_json, '$.requestId')
-                ORDER BY activity.created_at DESC, activity.activity_id DESC
-              ) AS row_number
-            FROM projection_thread_activities AS activity
-            WHERE activity.thread_id = projection_threads.thread_id
-              AND json_extract(activity.payload_json, '$.requestId') IS NOT NULL
-              AND activity.kind IN (
-                'approval.requested',
-                'approval.resolved',
-                'provider.approval.respond.failed'
-              )
-          ) AS latest
-          WHERE latest.row_number = 1
-        )
         SELECT COUNT(*)
-        FROM latest_approval_states
-        WHERE latest_approval_states.kind = 'approval.requested'
-          OR (
-            latest_approval_states.kind = 'provider.approval.respond.failed'
-            AND latest_approval_states.detail NOT LIKE '%stale pending approval request%'
-            AND latest_approval_states.detail NOT LIKE '%unknown pending approval request%'
-            AND latest_approval_states.detail NOT LIKE '%unknown pending permission request%'
-          )
+        FROM projection_pending_approvals
+        WHERE projection_pending_approvals.thread_id = projection_threads.thread_id
+          AND projection_pending_approvals.status = 'pending'
       ), 0),
       pending_user_input_count = COALESCE((
         WITH latest_user_input_states AS (

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit b6bcd14. Configure here.

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp bot commented Apr 13, 2026

Approvability

Verdict: Needs human review

This PR modifies runtime behavior in the approval system by automatically resolving pending approvals when stale/unknown failure events occur, and includes a data migration that backfills existing records. While the changes are well-tested, they affect how the approval workflow operates and warrant human review.

No code changes detected at 232001d. Prior analysis still applies.

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

@juliusmarminge
Copy link
Copy Markdown
Member Author

@cursor push 25f8053

cursoragent and others added 3 commits April 13, 2026 23:43
…n pending count

- ProjectionPipeline: add early return when existingRow is already resolved
  in the stale failure code path, preventing decision from being overwritten
  with null on already-resolved approvals.

- Migration step 4: add AND status = 'pending' condition so stale failure
  backfill does not overwrite rows already resolved by earlier steps.

- Migration step 5: replace activity-based pending_approval_count derivation
  with a direct count from projection_pending_approvals WHERE status = 'pending',
  consistent with the runtime's refreshThreadShellSummary.

Applied via @cursor push command
- Keep `provider.approval.respond.failed` events from clearing current pending approvals when they do not match the active request
- Add regression coverage for existing and missing approval rows
@juliusmarminge juliusmarminge merged commit c9b07d6 into main Apr 14, 2026
12 checks passed
@juliusmarminge juliusmarminge deleted the t3code/fix-thread-shell-mismatch branch April 14, 2026 01:10
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.

2 participants