Revert "fix(api): authorize owner_ids for list chats and search apps (#14775)#15698
Merged
Conversation
…nfiniflow#14775)" This reverts commit 5a5e766.
Contributor
📝 WalkthroughWalkthroughTwo REST endpoints ( ChangesTenant owner_ids authorization simplification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
Bot
added
the
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/apps/restful_apis/chat_api.py`:
- Around line 462-476: The code is passing request-supplied owner_ids directly
into DialogService.get_by_tenant_ids which bypasses server-side tenant
authorization; compute the caller's authorized tenant set by calling
UserTenantService.query(current_user.id) (or the existing helper that returns
joined_tenant_ids), validate and reject any owner_ids not in that set, then call
DialogService.get_by_tenant_ids with the authorized set (use the computed
joined_tenant_ids for both the branch that had owner_ids and the else branch) so
listing respects server-side scope; also add a debug/info log entry
(processLogger or request logger) noting the resolved tenant scope and any
rejected owner_ids for the new flow.
In `@api/apps/restful_apis/search_api.py`:
- Around line 87-95: The handler currently trusts request owner_ids and calls
SearchService.get_by_tenant_ids with unvalidated tenant IDs; instead validate
and compute the caller's authorized tenant IDs server-side (use
UserTenantService.query/current_user.id to fetch joined/authorized tenant IDs),
intersect or reject any requested owner_ids that are out-of-scope, then call
SearchService.get_by_tenant_ids exactly once with the validated tenant set and
apply paging only after that; also add logging for the authorization and
rejection flow (e.g., log requested owner_ids, authorized_tenants, and when a
request is rejected or adjusted).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b205d254-7e81-43b2-9f85-f532eb0810ad
📒 Files selected for processing (5)
api/apps/restful_apis/chat_api.pyapi/apps/restful_apis/search_api.pytest/testcases/test_http_api/test_chat_assistant_management/conftest.pytest/testcases/test_http_api/test_chat_assistant_management/test_chat_sdk_routes_unit.pytest/testcases/test_web_api/test_search_app/test_search_routes_unit.py
💤 Files with no reviewable changes (1)
- test/testcases/test_http_api/test_chat_assistant_management/test_chat_sdk_routes_unit.py
Comment on lines
462
to
+476
| if owner_ids: | ||
| requested_owner_ids = set(owner_ids) | ||
| unauthorized_owner_ids = requested_owner_ids - authorized_owner_ids | ||
| if unauthorized_owner_ids: | ||
| logging.warning( | ||
| "Rejected list_chats request: user=%s attempted unauthorized owner_ids=%s", | ||
| current_user.id, | ||
| sorted(unauthorized_owner_ids), | ||
| ) | ||
| return get_json_result( | ||
| data=False, | ||
| message="Only authorized owner_ids can be queried.", | ||
| code=RetCode.OPERATING_ERROR, | ||
| ) | ||
| effective_owner_ids = list(requested_owner_ids) | ||
| chats, total = await thread_pool_exec( | ||
| DialogService.get_by_tenant_ids, | ||
| owner_ids, current_user.id, 0, 0, orderby, desc, keywords, **exact_filters, | ||
| ) | ||
| chats = [chat for chat in chats if chat["tenant_id"] in owner_ids] | ||
| total = len(chats) | ||
| if page_number and items_per_page: | ||
| start = (page_number - 1) * items_per_page | ||
| chats = chats[start : start + items_per_page] | ||
| else: | ||
| effective_owner_ids = list(authorized_owner_ids) | ||
|
|
||
| chats, total = await thread_pool_exec( | ||
| DialogService.get_by_tenant_ids, | ||
| effective_owner_ids, current_user.id, page_number, items_per_page, orderby, desc, keywords, **exact_filters, | ||
| ) | ||
| chats, total = await thread_pool_exec( | ||
| DialogService.get_by_tenant_ids, | ||
| [], current_user.id, page_number, items_per_page, orderby, desc, keywords, **exact_filters, | ||
| ) |
Contributor
There was a problem hiding this comment.
Restore server-side owner authorization before querying.
DialogService.get_by_tenant_ids() treats its first argument as the authorized tenant scope (tenant_id IN joined_tenant_ids OR tenant_id == user_id in api/db/services/dialog_service.py:194-255). Passing request-supplied owner_ids straight through lets any authenticated caller enumerate chats for arbitrary tenants, and the else branch now regresses the default listing to only current_user.id chats even though get_chat still authorizes across UserTenantService.query() at Lines 489-497. Recompute the caller's joined-tenant set first, reject out-of-scope owner_ids, and use that authorized set for both filtered and unfiltered listing.
🔒 Proposed fix
try:
page_number = int(request.args.get("page", 0))
items_per_page = validate_rest_api_page_size(int(request.args.get("page_size", 0)))
+ joined_tenants = await thread_pool_exec(UserTenantService.query, user_id=current_user.id)
+ authorized_owner_ids = {current_user.id}
+ authorized_owner_ids.update(tenant.tenant_id for tenant in joined_tenants)
if owner_ids:
- chats, total = await thread_pool_exec(
- DialogService.get_by_tenant_ids,
- owner_ids, current_user.id, 0, 0, orderby, desc, keywords, **exact_filters,
- )
- chats = [chat for chat in chats if chat["tenant_id"] in owner_ids]
- total = len(chats)
- if page_number and items_per_page:
- start = (page_number - 1) * items_per_page
- chats = chats[start : start + items_per_page]
+ unauthorized_owner_ids = sorted(set(owner_ids) - authorized_owner_ids)
+ if unauthorized_owner_ids:
+ logging.warning(
+ "Rejecting unauthorized owner_ids on list_chats: user_id=%s owner_ids=%s",
+ current_user.id,
+ unauthorized_owner_ids,
+ )
+ return get_json_result(
+ data=False,
+ message="Please specify authorized owner_ids only.",
+ code=RetCode.OPERATING_ERROR,
+ )
+ effective_owner_ids = owner_ids
else:
- chats, total = await thread_pool_exec(
- DialogService.get_by_tenant_ids,
- [], current_user.id, page_number, items_per_page, orderby, desc, keywords, **exact_filters,
- )
+ effective_owner_ids = list(authorized_owner_ids)
+
+ chats, total = await thread_pool_exec(
+ DialogService.get_by_tenant_ids,
+ effective_owner_ids,
+ current_user.id,
+ page_number,
+ items_per_page,
+ orderby,
+ desc,
+ keywords,
+ **exact_filters,
+ )As per coding guidelines, "Add logging for new flows".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/apps/restful_apis/chat_api.py` around lines 462 - 476, The code is
passing request-supplied owner_ids directly into DialogService.get_by_tenant_ids
which bypasses server-side tenant authorization; compute the caller's authorized
tenant set by calling UserTenantService.query(current_user.id) (or the existing
helper that returns joined_tenant_ids), validate and reject any owner_ids not in
that set, then call DialogService.get_by_tenant_ids with the authorized set (use
the computed joined_tenant_ids for both the branch that had owner_ids and the
else branch) so listing respects server-side scope; also add a debug/info log
entry (processLogger or request logger) noting the resolved tenant scope and any
rejected owner_ids for the new flow.
Comment on lines
+87
to
+95
| if not owner_ids: | ||
| tenants = [] | ||
| search_apps, total = SearchService.get_by_tenant_ids(tenants, current_user.id, page_number, items_per_page, orderby, desc, keywords) | ||
| else: | ||
| effective_owner_ids = list(authorized_owner_ids) | ||
|
|
||
| search_apps, total = SearchService.get_by_tenant_ids( | ||
| effective_owner_ids, current_user.id, page_number, items_per_page, orderby, desc, keywords | ||
| ) | ||
| search_apps, total = SearchService.get_by_tenant_ids(owner_ids, current_user.id, 0, 0, orderby, desc, keywords) | ||
| search_apps = [s for s in search_apps if s["tenant_id"] in owner_ids] | ||
| total = len(search_apps) | ||
| if page_number and items_per_page: | ||
| search_apps = search_apps[(page_number - 1) * items_per_page: page_number * items_per_page] |
Contributor
There was a problem hiding this comment.
Do not treat request owner_ids as the authorized tenant set.
SearchService.get_by_tenant_ids() uses its first argument as joined_tenant_ids (api/db/services/search_service.py:83-116), so this rewrite lets any authenticated user list search apps for arbitrary tenants by supplying their ids here. It also changes the default path to [], which hides joined-tenant search apps even though detail() still authorizes through UserTenantService.query() at Lines 105-110. Resolve the caller's authorized tenant ids server-side, reject any out-of-scope owner_ids, and query once with that validated set.
🔒 Proposed fix
try:
- if not owner_ids:
- tenants = []
- search_apps, total = SearchService.get_by_tenant_ids(tenants, current_user.id, page_number, items_per_page, orderby, desc, keywords)
- else:
- search_apps, total = SearchService.get_by_tenant_ids(owner_ids, current_user.id, 0, 0, orderby, desc, keywords)
- search_apps = [s for s in search_apps if s["tenant_id"] in owner_ids]
- total = len(search_apps)
- if page_number and items_per_page:
- search_apps = search_apps[(page_number - 1) * items_per_page: page_number * items_per_page]
+ joined_tenants = UserTenantService.query(user_id=current_user.id)
+ authorized_owner_ids = {current_user.id}
+ authorized_owner_ids.update(tenant.tenant_id for tenant in joined_tenants)
+ if owner_ids:
+ unauthorized_owner_ids = sorted(set(owner_ids) - authorized_owner_ids)
+ if unauthorized_owner_ids:
+ logging.warning(
+ "Rejecting unauthorized owner_ids on list_searches: user_id=%s owner_ids=%s",
+ current_user.id,
+ unauthorized_owner_ids,
+ )
+ return get_json_result(
+ data=False,
+ message="Please specify authorized owner_ids only.",
+ code=RetCode.OPERATING_ERROR,
+ )
+ effective_owner_ids = owner_ids
+ else:
+ effective_owner_ids = list(authorized_owner_ids)
+
+ search_apps, total = SearchService.get_by_tenant_ids(
+ effective_owner_ids,
+ current_user.id,
+ page_number,
+ items_per_page,
+ orderby,
+ desc,
+ keywords,
+ )
return get_json_result(data={"search_apps": search_apps, "total": total})As per coding guidelines, "Add logging for new flows".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if not owner_ids: | |
| tenants = [] | |
| search_apps, total = SearchService.get_by_tenant_ids(tenants, current_user.id, page_number, items_per_page, orderby, desc, keywords) | |
| else: | |
| effective_owner_ids = list(authorized_owner_ids) | |
| search_apps, total = SearchService.get_by_tenant_ids( | |
| effective_owner_ids, current_user.id, page_number, items_per_page, orderby, desc, keywords | |
| ) | |
| search_apps, total = SearchService.get_by_tenant_ids(owner_ids, current_user.id, 0, 0, orderby, desc, keywords) | |
| search_apps = [s for s in search_apps if s["tenant_id"] in owner_ids] | |
| total = len(search_apps) | |
| if page_number and items_per_page: | |
| search_apps = search_apps[(page_number - 1) * items_per_page: page_number * items_per_page] | |
| try: | |
| joined_tenants = UserTenantService.query(user_id=current_user.id) | |
| authorized_owner_ids = {current_user.id} | |
| authorized_owner_ids.update(tenant.tenant_id for tenant in joined_tenants) | |
| if owner_ids: | |
| unauthorized_owner_ids = sorted(set(owner_ids) - authorized_owner_ids) | |
| if unauthorized_owner_ids: | |
| logging.warning( | |
| "Rejecting unauthorized owner_ids on list_searches: user_id=%s owner_ids=%s", | |
| current_user.id, | |
| unauthorized_owner_ids, | |
| ) | |
| return get_json_result( | |
| data=False, | |
| message="Please specify authorized owner_ids only.", | |
| code=RetCode.OPERATING_ERROR, | |
| ) | |
| effective_owner_ids = owner_ids | |
| else: | |
| effective_owner_ids = list(authorized_owner_ids) | |
| search_apps, total = SearchService.get_by_tenant_ids( | |
| effective_owner_ids, | |
| current_user.id, | |
| page_number, | |
| items_per_page, | |
| orderby, | |
| desc, | |
| keywords, | |
| ) | |
| return get_json_result(data={"search_apps": search_apps, "total": total}) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/apps/restful_apis/search_api.py` around lines 87 - 95, The handler
currently trusts request owner_ids and calls SearchService.get_by_tenant_ids
with unvalidated tenant IDs; instead validate and compute the caller's
authorized tenant IDs server-side (use UserTenantService.query/current_user.id
to fetch joined/authorized tenant IDs), intersect or reject any requested
owner_ids that are out-of-scope, then call SearchService.get_by_tenant_ids
exactly once with the validated tenant set and apply paging only after that;
also add logging for the authorization and rejection flow (e.g., log requested
owner_ids, authorized_tenants, and when a request is rejected or adjusted).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15698 +/- ##
=======================================
Coverage 93.16% 93.16%
=======================================
Files 10 10
Lines 717 717
Branches 118 118
=======================================
Hits 668 668
Misses 29 29
Partials 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/testcases/restful_api/test_chats.py (1)
1254-1283:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the normal pagination args on the omitted-
owner_idspath.This branch changed two behaviors in
list_chats: it now passes[]forowner_ids, and it keeps normal DB pagination. The test only captures the first argument, so it would still pass if the route regressed topage_number=0, items_per_page=0and fetched the full result set here.Suggested test tightening
- def _get_by_tenant_ids(owner_ids, *_args, **_kwargs): + def _get_by_tenant_ids(owner_ids, _user_id, page_number, items_per_page, *_args, **_kwargs): captured["owner_ids"] = owner_ids + captured["page_number"] = page_number + captured["items_per_page"] = items_per_page return ([], 0) monkeypatch.setattr(module.DialogService, "get_by_tenant_ids", _get_by_tenant_ids) res = _run(module.list_chats.__wrapped__()) assert res["code"] == 0 assert captured["owner_ids"] == [] + assert captured["page_number"] == 1 + assert captured["items_per_page"] == 10As per coding guidelines, "Add/adjust tests for behavior changes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/testcases/restful_api/test_chats.py` around lines 1254 - 1283, The test currently only captures owner_ids from DialogService.get_by_tenant_ids; update the test_list_chats_passes_empty_owner_ids_when_omitted_unit to also capture and assert the pagination args passed to module.DialogService.get_by_tenant_ids (e.g., capture page_number and items_per_page in the _get_by_tenant_ids stub) and add assertions that they equal the expected values (page_number==1 and items_per_page==10) when calling module.list_chats.__wrapped__(), while keeping the existing check that captured["owner_ids"] == [].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/testcases/restful_api/test_chats.py`:
- Around line 1287-1320: The test must verify the new manual-pagination
contract: have the mocked DialogService.get_by_tenant_ids (used by list_chats)
be called with offset=0 and limit=0 and return the full unpaginated list so
list_chats does in-memory filter+slice; modify the _get_by_tenant_ids stub to
capture owner_ids, offset and limit (e.g., captured["owner_ids"],
captured["offset"], captured["limit"]), ignore any incoming offset/limit args
and return both team_chat and own_chat plus total 2, then assert
captured["owner_ids"] == ["team-tenant-2"], captured["offset"] == 0 and
captured["limit"] == 0 and that the final response contains only ["team-chat"]
and total 1 to prove filtering+manual pagination occurred.
---
Outside diff comments:
In `@test/testcases/restful_api/test_chats.py`:
- Around line 1254-1283: The test currently only captures owner_ids from
DialogService.get_by_tenant_ids; update the
test_list_chats_passes_empty_owner_ids_when_omitted_unit to also capture and
assert the pagination args passed to module.DialogService.get_by_tenant_ids
(e.g., capture page_number and items_per_page in the _get_by_tenant_ids stub)
and add assertions that they equal the expected values (page_number==1 and
items_per_page==10) when calling module.list_chats.__wrapped__(), while keeping
the existing check that captured["owner_ids"] == [].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3076c8e-18b4-4545-a50f-5ade55865108
📒 Files selected for processing (1)
test/testcases/restful_api/test_chats.py
Comment on lines
+1287
to
+1320
| def test_list_chats_filters_by_requested_owner_ids_unit(monkeypatch): | ||
| module = _load_chat_routes_unit_module(monkeypatch) | ||
| captured = {} | ||
| monkeypatch.setattr( | ||
| module, | ||
| "request", | ||
| SimpleNamespace( | ||
| args=SimpleNamespace( | ||
| get=lambda key, default=None: { | ||
| "keywords": "", | ||
| "page": "0", | ||
| "page_size": "0", | ||
| "page": "1", | ||
| "page_size": "10", | ||
| "orderby": "create_time", | ||
| "desc": "true", | ||
| "id": None, | ||
| "name": None, | ||
| }.get(key, default), | ||
| getlist=lambda key: ["foreign-tenant-id"] if key == "owner_ids" else [], | ||
| getlist=lambda key: ["team-tenant-2"] if key == "owner_ids" else [], | ||
| ) | ||
| ), | ||
| ) | ||
|
|
||
| def _get_by_tenant_ids(owner_ids, *_args, **_kwargs): | ||
| captured["owner_ids"] = owner_ids | ||
| team_chat = _DummyDialogRecord({"id": "team-chat", "tenant_id": "team-tenant-2", "name": "team"}).to_dict() | ||
| own_chat = _DummyDialogRecord({"id": "own-chat", "tenant_id": "tenant-1", "name": "own"}).to_dict() | ||
| return ([team_chat, own_chat], 2) | ||
|
|
||
| monkeypatch.setattr(module.DialogService, "get_by_tenant_ids", _get_by_tenant_ids) | ||
| res = _run(module.list_chats.__wrapped__()) | ||
| assert res["code"] == module.RetCode.OPERATING_ERROR | ||
| assert "authorized owner_ids" in res["message"] | ||
| assert res["code"] == 0 | ||
| assert captured["owner_ids"] == ["team-tenant-2"] | ||
| assert [chat["id"] for chat in res["data"]["chats"]] == ["team-chat"] | ||
| assert res["data"]["total"] == 1 |
Contributor
There was a problem hiding this comment.
Cover the new owner_ids manual-pagination contract, not just filtering.
When owner_ids is present, list_chats now intentionally calls DialogService.get_by_tenant_ids(..., 0, 0, ...), filters in memory, then slices the filtered list. With page=1, page_size=10, and only one matching row, this test still passes if the route falls back to normal DB pagination or skips the 0,0 call entirely.
Suggested test tightening
get=lambda key, default=None: {
"keywords": "",
- "page": "1",
- "page_size": "10",
+ "page": "2",
+ "page_size": "1",
"orderby": "create_time",
"desc": "true",
"id": None,
"name": None,
}.get(key, default),
getlist=lambda key: ["team-tenant-2"] if key == "owner_ids" else [],
)
),
)
- def _get_by_tenant_ids(owner_ids, *_args, **_kwargs):
+ def _get_by_tenant_ids(owner_ids, _user_id, page_number, items_per_page, *_args, **_kwargs):
captured["owner_ids"] = owner_ids
- team_chat = _DummyDialogRecord({"id": "team-chat", "tenant_id": "team-tenant-2", "name": "team"}).to_dict()
+ captured["page_number"] = page_number
+ captured["items_per_page"] = items_per_page
+ team_chat_1 = _DummyDialogRecord({"id": "team-chat-1", "tenant_id": "team-tenant-2", "name": "team-1"}).to_dict()
own_chat = _DummyDialogRecord({"id": "own-chat", "tenant_id": "tenant-1", "name": "own"}).to_dict()
- return ([team_chat, own_chat], 2)
+ team_chat_2 = _DummyDialogRecord({"id": "team-chat-2", "tenant_id": "team-tenant-2", "name": "team-2"}).to_dict()
+ return ([team_chat_1, own_chat, team_chat_2], 3)
monkeypatch.setattr(module.DialogService, "get_by_tenant_ids", _get_by_tenant_ids)
res = _run(module.list_chats.__wrapped__())
assert res["code"] == 0
assert captured["owner_ids"] == ["team-tenant-2"]
- assert [chat["id"] for chat in res["data"]["chats"]] == ["team-chat"]
- assert res["data"]["total"] == 1
+ assert captured["page_number"] == 0
+ assert captured["items_per_page"] == 0
+ assert [chat["id"] for chat in res["data"]["chats"]] == ["team-chat-2"]
+ assert res["data"]["total"] == 2As per coding guidelines, "Add/adjust tests for behavior changes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/testcases/restful_api/test_chats.py` around lines 1287 - 1320, The test
must verify the new manual-pagination contract: have the mocked
DialogService.get_by_tenant_ids (used by list_chats) be called with offset=0 and
limit=0 and return the full unpaginated list so list_chats does in-memory
filter+slice; modify the _get_by_tenant_ids stub to capture owner_ids, offset
and limit (e.g., captured["owner_ids"], captured["offset"], captured["limit"]),
ignore any incoming offset/limit args and return both team_chat and own_chat
plus total 2, then assert captured["owner_ids"] == ["team-tenant-2"],
captured["offset"] == 0 and captured["limit"] == 0 and that the final response
contains only ["team-chat"] and total 1 to prove filtering+manual pagination
occurred.
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
1 participant
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.
This reverts PR #14775 commit 5a5e766.