Task redesign

[harshach]

Describe your changes:

Fixes

I worked on ... because ...

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

---

---

Summary by Gitar

• Test Maintenance:
  • Marked ActivityFeed mention notification tests as test.fixme to track resolution of pending issues.
  • Disabled several WorkflowDefinitionResourceIT integration tests pending fixes related to the task redesign scope.

_{This will update automatically on new commits.}

[github-actions]

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

[github-actions]

TypeScript types have been updated based on the JSON schema changes in the PR

[github-actions]

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[github-actions]

✅ TypeScript Types Auto-Updated

The generated TypeScript types have been automatically updated based on JSON schema changes in this PR.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

[gitar-bot]

Code Review ⚠️ Changes requested 30 resolved / 35 findings

Redesign of the task workflow addresses numerous critical stability, security, and performance defects, including SQL injections and N+1 query issues. Further attention is required to resolve open findings regarding MWAA pagination, SLF4J logging, auth polling cleanup, endpoint authorization, and table key validation.

⚠️ Edge Case: MWAA client _paginate uses total_entries which MWAA may not return

In mwaa.py line 140 and client.py line 224, the pagination logic sets total = response.get('total_entries', len(result)). If the MWAA invoke_rest_api response doesn't include total_entries (which is not guaranteed for all MWAA versions/configurations), the fallback len(result) will always equal offset + len(page) after extending, causing the loop to terminate after the first page when the page is smaller than limit. This means pagination will silently stop fetching after the first full page if the API returns results but omits total_entries.

Additionally, if a page returns exactly limit items but there are no more, the code would make one extra request that returns empty — this is acceptable. But the missing total_entries case could truncate results.

⚠️ Bug: SLF4J log messages missing space or placeholder for exception

Several String.format → SLF4J conversions have formatting issues:

1. Missing space before {}: UserResource.java:735 outputs "Error in sending invite to User<message>" and line 1361 outputs "Error in sending mail for reset password<message>" — both are missing a space (or colon+space) before {}.

2. Dangling "due to " without placeholder: In OpenMetadataConnectionBuilder.java:80, MigrationOps.java:28, v159/MigrationUtil.java:65, and v150/MigrationUtil.java:108, the message says "... due to " but has no {} for the exception. SLF4J will still log the stack trace (last arg treated as Throwable), but the message text reads incomplete — it should either be "due to: " or include "due to [{}]" with e.getMessage().

Suggested fix

// UserResource.java:735
LOG.error("Error in sending invite to User: {}", ex.getMessage());
// UserResource.java:1361
LOG.error("Error in sending mail for reset password: {}", ex.getMessage());
// OpenMetadataConnectionBuilder.java:79-82
LOG.warn(
    "Could not initialize bot for pipeline [{}] due to: ",
    ingestionPipeline.getPipelineType(),
    e);

⚠️ Security: listRetryQueue endpoint has no authorization check

The new listRetryQueue endpoint at /v1/apps/name/{name}/live-indexing-queue receives SecurityContext but never calls authorizer.authorize(). Other endpoints in the same class (e.g., scheduleApplication, triggerApplicationRun) properly authorize before performing operations. This means any authenticated user can view the retry queue, which may expose internal indexing failures and entity metadata that should be restricted to admins.

Suggested fix

Add an authorization check before accessing the retry queue:

OperationContext operationContext = new OperationContext(entityType, MetadataOperation.VIEW_ALL);
ResourceContext<?> resourceContext = getResourceContextByName(name);
authorizer.authorize(securityContext, operationContext, resourceContext);

⚠️ Bug: rowKey="name" assumes column exists in sample data rows

The <Table> component uses rowKey="name", but the dataSource rows are dynamically built from column names in getSampleDataWithType (lines 146-153). Each row object is keyed by actual column names (e.g., id, email, diffType). If the table schema doesn't include a column literally named name, every row will have undefined as its key, causing React duplicate-key warnings and potential rendering bugs (e.g., rows not updating correctly on re-render).

Suggested fix

Use the row index as key instead:

-      rowKey="name"
+      rowKey={(_, index) => String(index)}

💡 Bug: Auth login polling has no cleanup on unmount

The new polling mechanism in onLoginHandler uses recursive setTimeout (up to 100 × 50ms = 5s) to wait for authenticatorRef.current. If the component unmounts during polling (e.g. user navigates away), the timeout callbacks continue to fire and may call setApplicationLoading(false) on an unmounted component, causing React warnings. On max-attempts exhaustion, the user sees loading silently stop with no error feedback.

Suggested fix

Use a ref (e.g. `isMountedRef`) set to false in a
useEffect cleanup, and check it before each retry
and before calling setApplicationLoading:

  const isMounted = useRef(true);
  useEffect(() => () => { isMounted.current = false; }, []);
  // in invokeLogin:
  } else if (attempts < maxAttempts && isMounted.current) {
    ...
  } else if (isMounted.current) {
    setApplicationLoading(false);
  }

✅ 30 resolved

✅ Bug: domain filter parameter accepted but silently ignored

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/tasks/TaskResource.java:138 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/tasks/TaskResource.java:162 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ListFilter.java:179
The list endpoint at GET /v1/tasks declares a @QueryParam("domain") String domain parameter (line 138), but the filter-building logic (lines 162-190) never adds it to the ListFilter. The domain variable is accepted from the user but discarded, meaning requests like GET /v1/tasks?domain=myDomain will silently return unfiltered results.

All other filter parameters (status, category, type, priority, assignee, createdBy, aboutEntity, mentionedUser) are properly added to the filter, but domain is missing. This likely means domain-scoped task lists will show tasks from all domains, which breaks multi-tenancy expectations.

✅ Bug: applySuggestion in TaskWorkflowHandler only logs, never applies

📄 openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskWorkflowHandler.java:529 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/tasks/TaskResource.java:661 📄 openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskWorkflowHandler.java:134 📄 openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskWorkflowHandler.java:541
When a Suggestion-type task is resolved via the POST /v1/tasks/{id}/resolve endpoint, the flow goes through TaskWorkflowHandler.applyEntityChanges() → applySuggestion() (line 298 → line 529). However, the applySuggestion method at line 529-550 only extracts the payload and logs it — it never actually applies any changes to the entity.

Compare this to the dedicated PUT /v1/tasks/{id}/suggestion/apply endpoint, which uses SuggestionHandler.applySuggestion() and does correctly generate and apply patches to the target entity.

This means that resolving a Suggestion task through the standard resolve flow (which the bulk operation also uses for approve) will mark the task as "Approved" but silently skip applying the suggested changes to the entity. The user will see a successful resolution but no entity mutation occurs.

The fix should either delegate to SuggestionHandler.applySuggestion() or replicate the patch logic from FieldPathUtils/SuggestionHandler.

✅ Quality: updateSuggestion uses storeEntity bypassing change events and versioning

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/SuggestionsResource.java:475
In SuggestionsResource.updateSuggestion, the code changed from taskRepository.createOrUpdate(uriInfo, existingTask, userName) to taskRepository.storeEntity(existingTask, true). While this correctly persists the payload change, storeEntity is a raw DB write that bypasses version bumping, entity history archival, change event generation, and search index updates. This means suggestion edits won't appear in audit history, won't trigger webhooks/alerts, and the search index will have stale data. Consider using createOrUpdate or at minimum documenting why the lightweight path is acceptable here.

✅ Performance: N+1 queries and broken pagination in listMyAssignedTasks

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/tasks/TaskResource.java:305 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/tasks/TaskResource.java:320 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/tasks/TaskResource.java:306
The listMyAssignedTasks endpoint iterates over each assignee ID (user + all their teams) and issues a separate listInternal database query for each one. If a user belongs to 10 teams, this fires 11 separate queries.

Additionally:

1. Duplicate results: A task assigned to both the user directly AND their team will appear twice in the combined results, since there's no deduplication.
2. Broken pagination: The same before/after cursor is passed to each sub-query, so cursor-based pagination doesn't work correctly across the combined result set. The total count and paging metadata from the returned ResultList is also lost.
3. Limit exceeded: Each sub-query uses the user's limitParam, so the total results can be N * limitParam instead of just limitParam.

This should use a single query with an IN clause for all assignee IDs (user ID + team IDs), which the existing assigneeId-based ListFilter could be extended to support.

✅ Performance: Bulk suggestion operations load all tasks with unbounded limit

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/SuggestionsResource.java:359 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/SuggestionsResource.java:438 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/SuggestionsResource.java:382 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/SuggestionsResource.java:623
Both acceptAllSuggestions (line 359) and rejectAllSuggestions (line 438) in SuggestionsResource.java pass Integer.MAX_VALUE - 1 (~2.1 billion) as the page limit to taskRepository.listAfter(). This attempts to load every matching task into memory in a single query with no pagination. For deployments with many open suggestions, this could cause OOM errors or database timeouts. Additionally, new SuggestionHandler() is instantiated inside the loop on every iteration (lines 382, 461) despite being stateless.

...and 25 more resolved from earlier reviews

🤖 Prompt for agents

Code Review: Redesign of the task workflow addresses numerous critical stability, security, and performance defects, including SQL injections and N+1 query issues. Further attention is required to resolve open findings regarding MWAA pagination, SLF4J logging, auth polling cleanup, endpoint authorization, and table key validation.

1. ⚠️ Edge Case: MWAA client _paginate uses total_entries which MWAA may not return

   In `mwaa.py` line 140 and `client.py` line 224, the pagination logic sets `total = response.get('total_entries', len(result))`. If the MWAA `invoke_rest_api` response doesn't include `total_entries` (which is not guaranteed for all MWAA versions/configurations), the fallback `len(result)` will always equal `offset + len(page)` after extending, causing the loop to terminate after the first page when the page is smaller than `limit`. This means pagination will silently stop fetching after the first full page if the API returns results but omits `total_entries`.
   
   Additionally, if a page returns exactly `limit` items but there are no more, the code would make one extra request that returns empty — this is acceptable. But the missing `total_entries` case could truncate results.

2. ⚠️ Bug: SLF4J log messages missing space or placeholder for exception

   Several `String.format` → SLF4J conversions have formatting issues:
   
   1. **Missing space before `{}`**: `UserResource.java:735` outputs `"Error in sending invite to User<message>"` and line 1361 outputs `"Error in sending mail for reset password<message>"` — both are missing a space (or colon+space) before `{}`.
   
   2. **Dangling "due to " without placeholder**: In `OpenMetadataConnectionBuilder.java:80`, `MigrationOps.java:28`, `v159/MigrationUtil.java:65`, and `v150/MigrationUtil.java:108`, the message says `"... due to "` but has no `{}` for the exception. SLF4J will still log the stack trace (last arg treated as Throwable), but the message text reads incomplete — it should either be `"due to: "` or include `"due to [{}]"` with `e.getMessage()`.

   Suggested fix:
   // UserResource.java:735
   LOG.error("Error in sending invite to User: {}", ex.getMessage());
   // UserResource.java:1361
   LOG.error("Error in sending mail for reset password: {}", ex.getMessage());
   // OpenMetadataConnectionBuilder.java:79-82
   LOG.warn(
       "Could not initialize bot for pipeline [{}] due to: ",
       ingestionPipeline.getPipelineType(),
       e);

3. 💡 Bug: Auth login polling has no cleanup on unmount

   The new polling mechanism in `onLoginHandler` uses recursive `setTimeout` (up to 100 × 50ms = 5s) to wait for `authenticatorRef.current`. If the component unmounts during polling (e.g. user navigates away), the timeout callbacks continue to fire and may call `setApplicationLoading(false)` on an unmounted component, causing React warnings. On max-attempts exhaustion, the user sees loading silently stop with no error feedback.

   Suggested fix:
   Use a ref (e.g. `isMountedRef`) set to false in a
   useEffect cleanup, and check it before each retry
   and before calling setApplicationLoading:
   
     const isMounted = useRef(true);
     useEffect(() => () => { isMounted.current = false; }, []);
     // in invokeLogin:
     } else if (attempts < maxAttempts && isMounted.current) {
       ...
     } else if (isMounted.current) {
       setApplicationLoading(false);
     }

4. ⚠️ Security: listRetryQueue endpoint has no authorization check

   The new `listRetryQueue` endpoint at `/v1/apps/name/{name}/live-indexing-queue` receives `SecurityContext` but never calls `authorizer.authorize()`. Other endpoints in the same class (e.g., `scheduleApplication`, `triggerApplicationRun`) properly authorize before performing operations. This means any authenticated user can view the retry queue, which may expose internal indexing failures and entity metadata that should be restricted to admins.

   Suggested fix:
   Add an authorization check before accessing the retry queue:
   
   OperationContext operationContext = new OperationContext(entityType, MetadataOperation.VIEW_ALL);
   ResourceContext<?> resourceContext = getResourceContextByName(name);
   authorizer.authorize(securityContext, operationContext, resourceContext);

5. ⚠️ Bug: rowKey="name" assumes column exists in sample data rows

   The `<Table>` component uses `rowKey="name"`, but the `dataSource` rows are dynamically built from column names in `getSampleDataWithType` (lines 146-153). Each row object is keyed by actual column names (e.g., `id`, `email`, `diffType`). If the table schema doesn't include a column literally named `name`, every row will have `undefined` as its key, causing React duplicate-key warnings and potential rendering bugs (e.g., rows not updating correctly on re-render).

   Suggested fix:
   Use the row index as key instead:
   
   -      rowKey="name"
   +      rowKey={(_, index) => String(index)}

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

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

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
1058 New issues
0 Accepted issues

Measures
17 Security Hotspots
0.0% Coverage on New Code
3.9% Duplication on New Code

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
19 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[github-actions]

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

✅ 3924 passed · ❌ 2 failed · 🟡 19 flaky · ⏭️ 96 skipped

Shard	Passed	Failed	Flaky	Skipped
🔴 Shard 1	488	1	1	4
🟡 Shard 2	697	0	7	8
🔴 Shard 3	698	1	3	7
🟡 Shard 4	683	0	4	27
🟡 Shard 5	660	0	1	42
🟡 Shard 6	698	0	3	8

Genuine Failures (failed on all attempts)

❌ Pages/SearchIndexApplication.spec.ts › Search Index Application (shard 1)

Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoEqual�[2m(�[22m�[32mexpected�[39m�[2m) // deep equality�[22m

Expected: �[32mStringMatching /success|activeError/g�[39m
Received: �[31m"failed"�[39m

❌ Features/LandingPageWidgets/DomainWidgetFilter.spec.ts › Domains widget should show only selected domain when domain filter is active (shard 3)

�[31mTest timeout of 60000ms exceeded.�[39m

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

• Pages/Customproperties-part1.spec.ts › no duplicate card after update (shard 1, 1 retry)
• Features/ActivityAPI.spec.ts › Activity event shows the actor who made the change (shard 2, 1 retry)
• Features/CuratedAssets.spec.ts › Test Dashboards with display name filter (shard 2, 1 retry)
• Features/DataQuality/DataQualityDashboard.spec.ts › Dimension card click should redirect to test cases with applied filters (shard 2, 1 retry)
• Features/DomainFilterQueryFilter.spec.ts › Quick filters should persist when domain filter is applied and cleared (shard 2, 1 retry)
• Features/ExploreQuickFilters.spec.ts › tier with assigned asset appears in dropdown, tier without asset does not (shard 2, 1 retry)
• Features/Glossary/GlossaryWorkflow.spec.ts › should display correct status badge color and icon (shard 2, 2 retries)
• Features/Glossary/GlossaryWorkflow.spec.ts › should start term as Draft when glossary has reviewers (shard 2, 1 retry)
• Features/LandingPageWidgets/DomainWidgetFilter.spec.ts › Setup Domains widget on landing page (shard 3, 2 retries)
• Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
• Features/Table.spec.ts › Tags term should be consistent for search (shard 3, 1 retry)
• Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
• Pages/DataContracts.spec.ts › Create Data Contract and validate for Table (shard 4, 1 retry)
• Pages/Entity.spec.ts › Delete Dashboard Data Model (shard 4, 1 retry)
• Pages/Entity.spec.ts › Announcement create, edit & delete (shard 4, 1 retry)
• Pages/Entity.spec.ts › Tier Add, Update and Remove (shard 5, 1 retry)
• Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Directory (shard 6, 1 retry)
• Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for topic -> apiEndpoint (shard 6, 1 retry)
• Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 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