fix(dq): stale incident status on the Test Case page after reopening a resolved incident

[manerow]

Fixes #30168

The mental model you need first

When a Data Quality test fails, the system opens an incident: a tracked problem someone has to work. An incident has a lifecycle (New -> Acknowledged -> Assigned -> Resolved) and is identified by a stateId. On main, each incident is also a Task entity (a real work item with an assignee, driven by a workflow engine); the Task's id is the incident's stateId.

Two separate things get written as a test lives its life:

1. Test results: every run writes a row (pass/fail). When a run fails, that row is stamped with the id of the incident it belongs to (result.incidentId). This stamp is written once, at the moment the result is ingested, and never touched again.
2. Incident status records: every status change (New/Ack/Assigned/Resolved) writes a row to a separate timeline, all sharing the incident's stateId.

The Test Case page needs to answer one question: "Does this test case have an open incident right now, and what's its status?" It reads a field called TestCase.incidentId to do that. That field is the whole story.

The problem (what the customer saw)

Someone resolved an incident by mistake, then set it back to Acknowledged. The incident itself showed Acknowledged correctly, but the Test Case page kept showing Resolved, forever, until the test failed again.

Why it happened (how it worked before)

TestCase.incidentId was computed from the wrong source, and inconsistently:

• The list pages computed it from the latest test result's stamp (result.incidentId). But that stamp is frozen at ingestion time. It answers "which incident did the last run belong to," which is not the same question as "what is the current incident."
• The detail page read the incident timeline, but returned the latest incident's id even if it was resolved.

So the field didn't reliably mean "the current open incident." It meant "whatever incident the last failing run was stamped with" (list) or "the latest incident, resolved or not" (detail). These two disagreed with each other, and both drifted from reality.

The reopen scenario is where it breaks visibly:

1. Test fails -> incident S1 opens, the failed result is stamped incidentId = S1.
2. Ack, then Resolved -> all still on S1.
3. User reopens with Ack. Resolved is a terminal state, so the system mints a brand-new incident S2. But no test result points to S2: the last result is still stamped S1.

Now TestCase.incidentId (from the frozen result stamp) still says S1, and S1's latest status is Resolved. So the page shows Resolved, even though the live incident is S2/Ack. It only "self-heals" when the test fails again, because that writes a new result stamped with the current incident.

On main there was an extra layer of broken: because of a latent bug (the code that looked up the incident's Task by id never actually loaded the field it filtered on), reopening didn't just mis-point the field, it created a phantom incident with no Task at all, outside the workflow (no assignee flow, no notifications).

What the PR changes

1. One definition of "current incident," used everywhere. TestCase.incidentId now always means: the stateId of the latest unresolved incident record, or null if there's no open incident. Both the single-read and the bulk (list) paths now compute it the same way, from the incident timeline. The frozen result stamp is no longer consulted for this. This is the core fix.
2. Reopening actually reopens the same incident. A manual Resolved -> Ack/Assigned now keeps the same stateId and reopens the same Task (the workflow is restarted on it), instead of minting a phantom. A genuinely new failure after a resolution still starts a fresh incident; that path is untouched. The distinction is deliberate: a human un-resolving is correcting the incident; a new failure is a new problem.
3. The denormalized copy is kept honest. There's a stored copy of incidentId on the test case row (for reads that don't request the field). The PR updates it on every incident status change (set to the incident id while active, cleared to null on resolve) so it stays aligned with the current incident.
4. The UI stops working around the backend. The status modal used to reopen by creating a new Task on the client side. Now it just posts the status and lets the fixed backend reopen the existing incident.

How it works now (same scenario)

1. Test fails -> incident S1, page shows the open incident.
2. Ack -> S1, page shows Ack.
3. Resolved -> TestCase.incidentId becomes null -> page shows No Incident (correct: nothing is open).
4. Reopen with Ack -> the same S1 reopens (same Task, back to live at the Ack stage) -> TestCase.incidentId = S1 -> page shows Ack.

Verified on a live deployment: after step 4 the incident id is the same S1, the Task is back to InProgress at the ack stage, and the timeline reads New -> Ack -> Resolved -> New -> Ack under one id.

Why this is the right way

• Single source of truth. "Current incident" is a fact about the incident's own lifecycle, so it's read from the incident timeline, never inferred from result telemetry or a frozen stamp. Detail and list pages can no longer disagree, and there's no "self-heals on next failure" staleness class left.
• It matches the product's own model. The product treats incidents as human-managed work items: a passing test does not auto-close an incident; a person must resolve it. Deriving the pointer from "the latest run's stamp" quietly contradicted that. Reading "the latest unresolved incident" is that model.
• Reopen = same incident is the standard. PagerDuty, ServiceNow, and Jira all reopen the same record when a human un-resolves; they don't fork a new one. The old behavior forked a phantom with no work item attached, which is both non-standard and where the evidence (the failing runs) got orphaned. Reusing the id keeps one coherent incident with its history and its Task intact.

Two intended, correct side effects fall out of this: after a resolve the page shows "No Incident" instead of a stale Resolved chip, and a currently-passing test that still has an unresolved incident now correctly shows it (because the incident is genuinely still open until someone resolves it).

Testing

• New IncidentTaskIntegrationIT tests: reopen keeps the stateId and returns the same Task to Open (Ack and Assigned variants), re-resolving completes that same Task, a new failure on a reopened incident reuses it, and there is never a second Task per incident.
• New TestCaseResourceIT#test_incidentIdDerivation_followsLatestUnresolvedTcrs: derivation matrix over the single GET and the bulk list (no incident -> null, New/Ack -> stateId, Resolved -> null, reopen -> same stateId, passing run while open -> stateId).
• Full IncidentTaskIntegrationIT and TestCaseResourceIT suites pass.
• Reproduced the customer scenario end to end on a live deployment before and after the fix: after Resolved -> Ack the test case page now shows Ack.

[github-actions]

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

[gitar-bot]

Code Review 👍 Approved with suggestions 3 resolved / 4 findings

Synchronizes incident status across Test Case pages by deriving the incident pointer from the resolution timeline rather than result stamps. Reopening resolved incidents now correctly reuses the original task, though the new restart detection logic requires monitoring for false-positive workflow failures.

💡 Edge Case: Reopen restart detection may false-positive on immediate workflow completion

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TaskRepository.java:887-901

reopenTaskWithWorkflow now treats a null workflowInstanceId on the refreshed task as a restart failure and rolls the task back to its terminal state, throwing IllegalStateException. This is correct when the trigger fails, but if a reopened governance workflow starts and then immediately reaches its end event (e.g. an auto-transition path), the running Flowable instance ends and workflowInstanceId can be cleared to null even though the restart succeeded — which would wrongly roll the reopened incident back to Resolved. If success is not guaranteed to leave a persisted, non-null workflowInstanceId, prefer keying failure detection off the explicit failure marker (workflow-start-failed stage) or an affirmative success signal rather than the absence of an instance id.

✅ 3 resolved

✅ Edge Case: Fallback reopen leaves denormalized row incidentId stale

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:327-336 📄 openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/handlers/IncidentTcrsSyncHandler.java:194-208
When reopenTaskWithWorkflow fails, applyLegacyStatusToIncidentTask returns false and storeInternal falls back to a direct TCRS insert under the resolved incident's stateId. Because no task stage transition occurs on this path, IncidentTcrsSyncHandler.updateTestCaseRowIncident never runs, so TestCase.incidentId on the row stays null (from the prior Resolved). Field-selecting reads still derive the correct stateId from the timeseries, but no-fields reads that rely on the denormalized row will show 'No Incident' for a reopened-but-taskless incident. Consider updating the row incidentId in the fallback branch too.

✅ Quality: Duplicated search-index field list risks silent drift

📄 openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/handlers/IncidentTcrsSyncHandler.java:89-92 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResultRepository.java:47-48
TEST_CASE_ROW_FIELDS in IncidentTcrsSyncHandler duplicates TestCaseResultRepository.TEST_CASE_INDEX_FIELDS verbatim, and the comment explicitly notes they must stay in sync so the row patch rebuilds the same search document. If one list is later updated, the search doc rebuilt via the sync handler will diverge silently. Reference the existing constant (make it public) instead of copying the string.

✅ Performance: Extra TCRS query on every non-failed result ingestion

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResultRepository.java:319-333
updateTestCaseStatus now calls resolveOngoingIncidentId, which for any non-Failed result issues an additional getLatestRecord query against the resolution-status timeseries on every result add/update. Previously the incidentId came directly from the in-memory result. In high-volume DQ ingestion this adds one indexed query per result; if it shows up in profiling, consider skipping the lookup when the test case row already reflects no open incident.

🤖 Prompt for agents

Code Review: Synchronizes incident status across Test Case pages by deriving the incident pointer from the resolution timeline rather than result stamps. Reopening resolved incidents now correctly reuses the original task, though the new restart detection logic requires monitoring for false-positive workflow failures.

1. 💡 Edge Case: Reopen restart detection may false-positive on immediate workflow completion
   Files: openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TaskRepository.java:887-901

   reopenTaskWithWorkflow now treats a null workflowInstanceId on the refreshed task as a restart failure and rolls the task back to its terminal state, throwing IllegalStateException. This is correct when the trigger fails, but if a reopened governance workflow starts and then immediately reaches its end event (e.g. an auto-transition path), the running Flowable instance ends and workflowInstanceId can be cleared to null even though the restart succeeded — which would wrongly roll the reopened incident back to Resolved. If success is not guaranteed to leave a persisted, non-null workflowInstanceId, prefer keying failure detection off the explicit failure marker (workflow-start-failed stage) or an affirmative success signal rather than the absence of an instance id.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	65.01% (75534/116176)	48.84% (45038/92206)	49.63% (13619/27441)

[gitar-bot]

Code Review ✅ Approved 4 resolved / 4 findings

Consistent incident state tracking is now enforced by deriving status from the latest unresolved resolution record rather than ingestion-time snapshots. This change correctly synchronizes incident lifecycles, resolves stale status displays after reopening, and ensures workflow consistency across task transitions.

✅ 4 resolved

✅ Edge Case: Fallback reopen leaves denormalized row incidentId stale

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:327-336 📄 openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/handlers/IncidentTcrsSyncHandler.java:194-208
When reopenTaskWithWorkflow fails, applyLegacyStatusToIncidentTask returns false and storeInternal falls back to a direct TCRS insert under the resolved incident's stateId. Because no task stage transition occurs on this path, IncidentTcrsSyncHandler.updateTestCaseRowIncident never runs, so TestCase.incidentId on the row stays null (from the prior Resolved). Field-selecting reads still derive the correct stateId from the timeseries, but no-fields reads that rely on the denormalized row will show 'No Incident' for a reopened-but-taskless incident. Consider updating the row incidentId in the fallback branch too.

✅ Quality: Duplicated search-index field list risks silent drift

📄 openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/handlers/IncidentTcrsSyncHandler.java:89-92 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResultRepository.java:47-48
TEST_CASE_ROW_FIELDS in IncidentTcrsSyncHandler duplicates TestCaseResultRepository.TEST_CASE_INDEX_FIELDS verbatim, and the comment explicitly notes they must stay in sync so the row patch rebuilds the same search document. If one list is later updated, the search doc rebuilt via the sync handler will diverge silently. Reference the existing constant (make it public) instead of copying the string.

✅ Performance: Extra TCRS query on every non-failed result ingestion

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResultRepository.java:319-333
updateTestCaseStatus now calls resolveOngoingIncidentId, which for any non-Failed result issues an additional getLatestRecord query against the resolution-status timeseries on every result add/update. Previously the incidentId came directly from the in-memory result. In high-volume DQ ingestion this adds one indexed query per result; if it shows up in profiling, consider skipping the lookup when the test case row already reflects no open incident.

✅ Edge Case: Reopen restart detection may false-positive on immediate workflow completion

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TaskRepository.java:887-901
reopenTaskWithWorkflow now treats a null workflowInstanceId on the refreshed task as a restart failure and rolls the task back to its terminal state, throwing IllegalStateException. This is correct when the trigger fails, but if a reopened governance workflow starts and then immediately reaches its end event (e.g. an auto-transition path), the running Flowable instance ends and workflowInstanceId can be cleared to null even though the restart succeeded — which would wrongly roll the reopened incident back to Resolved. If success is not guaranteed to leave a persisted, non-null workflowInstanceId, prefer keying failure detection off the explicit failure marker (workflow-start-failed stage) or an affirmative success signal rather than the absence of an instance id.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[gitar-bot]

Code Review ✅ Approved 4 resolved / 4 findings

Consistent incident state tracking is now enforced by deriving status from the latest unresolved resolution record rather than ingestion-time snapshots. This change correctly synchronizes incident lifecycles, resolves stale status displays after reopening, and ensures workflow consistency across task transitions.

✅ 4 resolved

✅ Edge Case: Fallback reopen leaves denormalized row incidentId stale

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:327-336 📄 openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/handlers/IncidentTcrsSyncHandler.java:194-208
When reopenTaskWithWorkflow fails, applyLegacyStatusToIncidentTask returns false and storeInternal falls back to a direct TCRS insert under the resolved incident's stateId. Because no task stage transition occurs on this path, IncidentTcrsSyncHandler.updateTestCaseRowIncident never runs, so TestCase.incidentId on the row stays null (from the prior Resolved). Field-selecting reads still derive the correct stateId from the timeseries, but no-fields reads that rely on the denormalized row will show 'No Incident' for a reopened-but-taskless incident. Consider updating the row incidentId in the fallback branch too.

✅ Quality: Duplicated search-index field list risks silent drift

📄 openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/handlers/IncidentTcrsSyncHandler.java:89-92 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResultRepository.java:47-48
TEST_CASE_ROW_FIELDS in IncidentTcrsSyncHandler duplicates TestCaseResultRepository.TEST_CASE_INDEX_FIELDS verbatim, and the comment explicitly notes they must stay in sync so the row patch rebuilds the same search document. If one list is later updated, the search doc rebuilt via the sync handler will diverge silently. Reference the existing constant (make it public) instead of copying the string.

✅ Performance: Extra TCRS query on every non-failed result ingestion

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResultRepository.java:319-333
updateTestCaseStatus now calls resolveOngoingIncidentId, which for any non-Failed result issues an additional getLatestRecord query against the resolution-status timeseries on every result add/update. Previously the incidentId came directly from the in-memory result. In high-volume DQ ingestion this adds one indexed query per result; if it shows up in profiling, consider skipping the lookup when the test case row already reflects no open incident.

✅ Edge Case: Reopen restart detection may false-positive on immediate workflow completion

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TaskRepository.java:887-901
reopenTaskWithWorkflow now treats a null workflowInstanceId on the refreshed task as a restart failure and rolls the task back to its terminal state, throwing IllegalStateException. This is correct when the trigger fails, but if a reopened governance workflow starts and then immediately reaches its end event (e.g. an auto-transition path), the running Flowable instance ends and workflowInstanceId can be cleared to null even though the restart succeeded — which would wrongly roll the reopened incident back to Resolved. If success is not guaranteed to leave a persisted, non-null workflowInstanceId, prefer keying failure detection off the explicit failure marker (workflow-start-failed stage) or an affirmative success signal rather than the absence of an instance id.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[greptile-apps]

Stale write remains possible. resolveOngoingIncidentId reads the TCRS timeline before the row update, with no lock or version check joining those operations. A result update can read incident X while it is open, then a concurrent resolution can append Resolved and clear the test-case row before this updater writes the previously read X back. No-fields reads then show the resolved incident as open. The timeline state must be validated atomically with the row write, or an older observation must be prevented from overwriting a newer transition.

[greptile-apps]

Pending reopen lacks recovery. This stores the task as Open with workflowInstanceId = null before starting Flowable. The rollback only runs after triggerWorkflowManagedTask returns false. If the process stops between the database write and the workflow call, the task remains Open in pending-workflow-start with no live workflow, and no recovery path shown here retries it. Later incident operations can reuse a task whose transitions, assignment, and notifications cannot run. The pending state needs restart reconciliation or another durable coordination mechanism.

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
1213 New issues
0 Accepted issues

Measures
13 Security Hotspots
0.0% Coverage on New Code
3.1% Duplication on New Code

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

🔴 Playwright Results — 2 failure(s), 29 flaky

✅ 4539 passed · ❌ 2 failed · 🟡 29 flaky · ⏭️ 96 skipped

Shard	Passed	Failed	Flaky	Skipped
🟡 Shard 1	437	0	3	16
✅ Shard 2	11	0	0	0
🔴 Shard 3	820	1	11	8
🟡 Shard 4	820	0	3	18
🟡 Shard 5	840	0	1	5
🟡 Shard 6	787	0	1	46
🔴 Shard 7	824	1	10	3

Genuine Failures (failed on all attempts)

❌ Features/ContextCenterArchive.spec.ts › archive page lazy-loads more rows on scroll within its own scroll container (shard 3)

Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoBeVisible�[2m(�[22m�[2m)�[22m failed

Locator: getByTestId('archive-view')
Expected: visible
Timeout: 15000ms
Error: element(s) not found

Call log:
�[2m  - Expect "toBeVisible" with timeout 15000ms�[22m
�[2m  - waiting for getByTestId('archive-view')�[22m

❌ Features/AutoPilot.spec.ts › Create Service and check the AutoPilot status (shard 7)

Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoBeVisible�[2m(�[22m�[2m)�[22m failed

Locator: getByText('AutoPilot agents run completed successfully.')
Expected: visible
Timeout: 60000ms
Error: element(s) not found

Call log:
�[2m  - Expect "toBeVisible" with timeout 60000ms�[22m
�[2m  - waiting for getByText('AutoPilot agents run completed successfully.')�[22m

🟡 29 flaky test(s) (passed on retry)

• Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: metric (shard 1, 1 retry)
• Pages/SearchSettings.spec.ts › Latest preview config wins when a superseded request resolves late (shard 1, 1 retry)
• Flow/SearchRBAC.spec.ts › a dashboard-scoped user sees dashboards but never tables (shard 1, 1 retry)
• Features/BulkEditEntity.spec.ts › Glossary Term (Nested) (shard 3, 2 retries)
• Features/BulkEditOperationBadges.spec.ts › Database service bulk edit search filters rows and clear restores them (shard 3, 1 retry)
• Features/BulkImportWithDotInName.spec.ts › Column with dot in name under service with dot (shard 3, 1 retry)
• Features/ContextCenterArticles.spec.ts › Article listing search filters, clears, and shows empty state (shard 3, 1 retry)
• Features/ContextCenterArticles.spec.ts › Article list cards, recently viewed widget, and pagination work (shard 3, 1 retry)
• Features/ContextCenterArticles.spec.ts › Left hierarchy pagination and expand collapse actions work (shard 3, 1 retry)
• Features/ContextCenterArticles.spec.ts › Article edit persistence and unsaved title behavior are correct (shard 3, 1 retry)
• Features/ContextCenterMemories.spec.ts › view modal switches to edit mode and saves changes (shard 3, 1 retry)
• Features/DataQuality/BundleSuiteBulkOperations.spec.ts › Add test case to existing Bundle Suite (shard 3, 1 retry)
• Features/DataQuality/TestCaseImportExportBasic.spec.ts › should show validation errors for invalid CSV (shard 3, 1 retry)
• Features/Glossary/GlossaryHierarchy.spec.ts › should move term with children to different glossary (shard 3, 1 retry)
• Features/Glossary/GlossaryTermRelationsGraphNested.spec.ts › viewing a child term: parent appears as a 1-hop neighbour via parentOf edge (shard 4, 1 retry)
• Features/IncidentManager.spec.ts › Complete Incident lifecycle with table owner (shard 4, 2 retries)
• Features/Table.spec.ts › should persist page size (shard 4, 1 retry)
• Pages/CustomProperties.spec.ts › Entity Reference List (shard 5, 1 retry)
• Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 6, 1 retry)
• Pages/Glossary.spec.ts › Drag and Drop Glossary Term (shard 7, 1 retry)
• Pages/GlossaryImportExport.spec.ts › Glossary Bulk Import Export (shard 7, 1 retry)
• Pages/GlossaryImportExport.spec.ts › Import partial success - some terms pass, some fail (shard 7, 1 retry)
• Pages/InputOutputPorts.spec.ts › Output ports section collapse/expand (shard 7, 1 retry)
• Pages/Lineage/LineageFilters.spec.ts › Verify Impact Analysis service filter selection (shard 7, 1 retry)
• Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab is NOT visible for pipelineService in platform lineage (shard 7, 1 retry)
• Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab is NOT visible for apiService in platform lineage (shard 7, 1 retry)
• Pages/TasksUIFlow.spec.ts › Create and reject tag task for Topic via UI (shard 7, 1 retry)
• Pages/TestSuite.spec.ts › Logical TestSuite (shard 7, 1 retry)
• Pages/Users.spec.ts › Check permissions for Data Steward (shard 7, 1 retry)

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace