fix(integrations): clear stale task status after connection disconnect (CS-166)#2577
Merged
Merged
Conversation
…t (CS-166) After a user disconnects an integration (e.g. GitHub), tasks that were marked 'failed' by that connection's check runs stayed stuck on 'failed' forever, because nothing re-evaluates the task's automation state on disconnect. In addition, the task detail "App Automations" panel kept showing historical failed runs from the disconnected connection, and the 12-hour task-schedule job would re-mark done-then-overdue tasks as failed using those stale runs. Three changes, all reading-side filters + one re-evaluation on disconnect: 1. ConnectionService.disconnectConnection and deleteConnection now call reevaluateFailedTasksAfterDisconnect, which re-derives each affected failed task's target status from its remaining non-disconnected automations. Uses a local mirror of the scheduler's getTargetStatus. 2. CheckRunRepository.findByTask excludes runs from disconnected connections so the UI no longer counts stale "failed" history. 3. task-schedule.ts includes the same connection-status filter on its integrationCheckRuns include. Check run rows are preserved for audit trail (matching the existing soft-delete design for IntegrationConnection itself). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/integration-platform/services/connection.service.ts">
<violation number="1" location="apps/api/src/integration-platform/services/connection.service.ts:148">
P2: Wrap `reevaluateFailedTasksAfterDisconnect` in a try-catch in both call sites. The connection is already disconnected when re-evaluation runs; if it throws, the primary operation should still succeed rather than surfacing an error to the caller for a best-effort side effect.</violation>
<violation number="2" location="apps/api/src/integration-platform/services/connection.service.ts:255">
P2: The `latestByCheckId` logic drops the `run.createdAt > existing.createdAt` guard present in the original `getTargetStatus`. This makes correctness silently dependent on the query's `orderBy` clause. Add the timestamp comparison to match the original's order-independent behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Two follow-ups to cubic's review on PR #2577: 1. Make the latest-run-per-checkId selection in deriveTargetStatusForTask order-independent: compare run.createdAt instead of trusting the caller's orderBy. Matches the scheduler's getTargetStatus. 2. Wrap reevaluateFailedTasksAfterDisconnect in try/catch at both call sites (disconnectConnection, deleteConnection). The primary disconnect has already persisted by the time re-evaluation runs; a transient DB error during this best-effort cleanup must not surface to the caller. Adds regression tests: (a) reverse-sorted input still picks newest run per checkId, (b) disconnect resolves successfully even when the runs query throws. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
🎉 This PR is included in version 3.23.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes CS-166 — after a customer disconnected their GitHub integration, related tasks were stuck on "failed" status and the task detail page kept showing historical failed automation runs.
Root cause
ConnectionService.disconnectConnection()soft-deletes the connection (setsstatus: 'disconnected', clears credentials) but does not touchTask.statusor theIntegrationCheckRunaudit rows. Three independent gaps result:Task.status = 'failed', nothing re-evaluates it. The task is pinned to failed indefinitely, because the 12-hour scheduler only processesdonetasks.CheckRunRepository.findByTaskreturned runs regardless of connection status, so the task detail panel still surfaced old failed runs under the active-integrations summary.apps/app/src/trigger/tasks/task/task-schedule.tspullsintegrationCheckRunswith no connection filter. For adonetask whose review window has elapsed, a stale failed run from the disconnected connection would flip the task tofailedon the next tick.Fix
Minimal, read-side filters + one re-evaluation on disconnect. No schema changes. Check run rows are preserved (matches the existing soft-delete design for
IntegrationConnection).ConnectionService.disconnectConnectionanddeleteConnectionnow callreevaluateFailedTasksAfterDisconnect(connectionId). For each failed task that had runs from this connection, it re-derives the target status from only active (non-disconnected) automations. Uses a local mirror of the scheduler'sgetTargetStatus— same 3-way logic (no automations →todo, all passing →done, elsefailed).CheckRunRepository.findByTaskfilters out runs whose connection isdisconnected.task-schedule.tsadds the same filter on itsintegrationCheckRunsinclude.Test plan
cd apps/api && npx jest src/integration-platform/services/connection.service→ 7/7 new tests pass:tododonefailedtodo; task detail "App Automations" panel no longer shows old runsdoneNotes
status: 'failed'. We don't demotedoneortodotasks — those transitions already have dedicated flows.ConnectionServicerather than extracted. The scheduler'sgetTargetStatuslives inapps/appand can't be imported fromapps/api; a shared package move would be a larger refactor than this bug requires.🤖 Generated with Claude Code
Summary by cubic
Clears stale “failed” task states when an integration (e.g., GitHub) is disconnected and hides runs from disconnected connections so the UI and scheduler ignore outdated failures. Fixes CS-166.
disconnectConnection/deleteConnection, re-derive each affected failed task’s status from active automations only (todoif none,doneif all passing; otherwise unchanged). Latest-per-checkIdselection is order-independent, and the cleanup is best-effort (wrapped in try/catch) so disconnect/delete still succeed. Audit rows stay.disconnectedconnections inCheckRunRepository.findByTaskandapps/app/src/trigger/tasks/task/task-schedule.tsso stale failures don’t affect the "App Automations" panel or scheduled status.Written for commit 9f140e1. Summary will update on new commits.